[PATCH] Fix POSIX compliance in pgwin32_unsetenv()
I noticed that pgwin32_unsetenv() in src/port/win32env.c lacks the input
validation that its sibling function pgwin32_setenv() has (lines 126-132).
Without these checks, the function will crash on NULL input via
strlen(NULL), and will accept empty strings or strings containing '=' in
violation of POSIX.1-2008.
The attached patch adds the same validation that pgwin32_setenv already
does, making the two functions consistent. This is purely defensive -
it only affects callers passing invalid arguments.
regards,
Bryan Green
Attachments:
0001-Fix-POSIX-compliance-in-pgwin32_unsetenv.patchtext/plain; charset=UTF-8; name=0001-Fix-POSIX-compliance-in-pgwin32_unsetenv.patchDownload
From dda02bef12a725eff5e38367f2a525b2355c29d0 Mon Sep 17 00:00:00 2001
From: Bryan Green <dbryan.green@gmail.com>
Date: Sat, 18 Oct 2025 13:04:04 -0500
Subject: [PATCH] Fix POSIX compliance in pgwin32_unsetenv()
pgwin32_unsetenv() lacks the input validation that its sibling
pgwin32_setenv() has. Add the same checks for NULL, empty string,
and '=' in the name parameter, per POSIX requirements.
Without these checks, unsetenv(NULL) crashes, and invalid names
are accepted when they should fail with EINVAL.
---
src/port/win32env.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/port/win32env.c b/src/port/win32env.c
index b22fbafde4..e1cee683db 100644
--- a/src/port/win32env.c
+++ b/src/port/win32env.c
@@ -152,6 +152,13 @@ pgwin32_unsetenv(const char *name)
int res;
char *envbuf;
+ /* Error conditions, per POSIX */
+ if (name == NULL || name[0] == '\0' || strchr(name, '=') != NULL)
+ {
+ errno = EINVAL;
+ return -1;
+ }
+
envbuf = (char *) malloc(strlen(name) + 2);
if (!envbuf)
return -1;
--
2.49.0
On Sat, Oct 18, 2025 at 01:26:40PM -0500, Bryan Green wrote:
I noticed that pgwin32_unsetenv() in src/port/win32env.c lacks the input
validation that its sibling function pgwin32_setenv() has (lines 126-132).Without these checks, the function will crash on NULL input via
strlen(NULL), and will accept empty strings or strings containing '=' in
violation of POSIX.1-2008.The attached patch adds the same validation that pgwin32_setenv already
does, making the two functions consistent. This is purely defensive - it
only affects callers passing invalid arguments.
I presume that you have tried to use this routine on some external
code on WIN32 to note that it was just crashing.
The current state of pgwin32_unsetenv() dates back to 0154345078fb.
The POSIX checks of setenv() are more recent than that, as in
7ca37fb0406b down to v14. I agree that the inconsistency in handling
the input arguments is annoying, so if there are no objections let's
apply the same checks down to v14 like the setenv() piece. It's
better than a hard crash.
--
Michael
On 10/19/25 20:02, Michael Paquier wrote:
On Sat, Oct 18, 2025 at 01:26:40PM -0500, Bryan Green wrote:
I noticed that pgwin32_unsetenv() in src/port/win32env.c lacks the input
validation that its sibling function pgwin32_setenv() has (lines 126-132).Without these checks, the function will crash on NULL input via
strlen(NULL), and will accept empty strings or strings containing '=' in
violation of POSIX.1-2008.The attached patch adds the same validation that pgwin32_setenv already
does, making the two functions consistent. This is purely defensive - it
only affects callers passing invalid arguments.I presume that you have tried to use this routine on some external
code on WIN32 to note that it was just crashing.The current state of pgwin32_unsetenv() dates back to 0154345078fb.
The POSIX checks of setenv() are more recent than that, as in
7ca37fb0406b down to v14. I agree that the inconsistency in handling
the input arguments is annoying, so if there are no objections let's
apply the same checks down to v14 like the setenv() piece. It's
better than a hard crash.
--
Michael
I have been going through all of the windows code line by line. That is
how I initially noticed this. I then wrote a program to exercise the
code and confirm the crash. I agree it should be backported.
BG
On Sun, Oct 19, 2025 at 08:34:02PM -0500, Bryan Green wrote:
I have been going through all of the windows code line by line. That is how
I initially noticed this. I then wrote a program to exercise the code and
confirm the crash. I agree it should be backported.
Applied down to v14.
Note that the unsetenv() WIN32 routine exists also in v13. However, I
could not get excited about it with the branch going EOL soon, also
knowing that 7ca37fb0406b affects v14~.
--
Michael