pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[PATCH] Fix POSIX compliance in pgwin32_unsetenv()
4+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2025-10-20 29b039e916 52b05b0686 199a0ecbcf 9666ce889e 4c3795c582 5274f11c17 Fix POSIX compliance in pgwin32_unsetenv() for "name" argument
* [PATCH] Fix POSIX compliance in pgwin32_unsetenv()
@ 2025-10-18 18:26 Bryan Green <dbryan.green@gmail.com>
2025-10-20 01:02 ` Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 4+ messages in thread
From: Bryan Green @ 2025-10-18 18:26 UTC (permalink / raw)
To: pgsql-hackers
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.patch (text/plain, 1.0K) download
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
* Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv()
2025-10-18 18:26 [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Bryan Green <dbryan.green@gmail.com>
@ 2025-10-20 01:02 ` Michael Paquier <michael@paquier.xyz>
2025-10-20 01:34 ` Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Bryan Green <dbryan.green@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Michael Paquier @ 2025-10-20 01:02 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: pgsql-hackers
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
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv()
2025-10-18 18:26 [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Bryan Green <dbryan.green@gmail.com>
2025-10-20 01:02 ` Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Michael Paquier <michael@paquier.xyz>
@ 2025-10-20 01:34 ` Bryan Green <dbryan.green@gmail.com>
2025-10-20 23:11 ` Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 4+ messages in thread
From: Bryan Green @ 2025-10-20 01:34 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv()
2025-10-18 18:26 [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Bryan Green <dbryan.green@gmail.com>
2025-10-20 01:02 ` Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Michael Paquier <michael@paquier.xyz>
2025-10-20 01:34 ` Re: [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Bryan Green <dbryan.green@gmail.com>
@ 2025-10-20 23:11 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2025-10-20 23:11 UTC (permalink / raw)
To: Bryan Green <dbryan.green@gmail.com>; +Cc: pgsql-hackers
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
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2025-10-20 23:11 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-10-18 18:26 [PATCH] Fix POSIX compliance in pgwin32_unsetenv() Bryan Green <dbryan.green@gmail.com>
2025-10-20 01:02 ` Michael Paquier <michael@paquier.xyz>
2025-10-20 01:34 ` Bryan Green <dbryan.green@gmail.com>
2025-10-20 23:11 ` Michael Paquier <michael@paquier.xyz>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox