On 10.10.24 14:59, Ranier Vilela wrote:
Please look at the SCRAM secret, which breaks parse_scram_secret(), perhaps because strsep() doesn't return NULL where strtok() did:CREATE ROLE r PASSWORD 'SCRAM- SHA-256$4096:hpFyHTUsSWcR7O9P$LgZFIt6Oqdo27ZFKbZ2nV+=vtnYM995pDh9ca6WSi120qVV5NeluNfUPkwm7Vqat25RjSPLkGeoZBQs6wVv+um4='; Core was generated by `postgres: law regression [local] CREATE ROLE '. Program terminated with signal SIGSEGV, Segmentation fault. #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:74 (gdb) bt #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:74 #1 0x0000563625e9e5b0 in parse_scram_secret (...) at auth-scram.c:655 Thanks for the report. It seems to me that it could be due to incorrect use of the strsep function. See:https://man7.org/linux/man-pages/man3/strsep.3.html <https://man7.org/ linux/man-pages/man3/strsep.3.html>" In case no delimiter was found, the token is taken to be the entire string/*stringp/, and/*stringp/ is made NULL. " So, it is necessary to check the *stringp* against NULL too.
Thanks for the analysis. I think moreover we *only* need to check the "stringp" for NULL, not the return value of strsep(), which would never be NULL in our case. So I propose the attached patch as a variant of yours.
From df4d43acb2956509e8166f2df4a3a2be1493ff6f Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Tue, 15 Oct 2024 08:39:30 +0200 Subject: [PATCH v2] Fix strsep() use for SCRAM secrets parsing The previous code (from commit 5d2e1cc117b) did not detect end of string correctly, so it would fail to error out if fewer than the expected number of fields were present, which could then later lead to a crash when NULL string pointers are accessed. Reported-by: Alexander Lakhin <exclus...@gmail.com> Reported-by: Ranier Vilela <ranier...@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/79692bf9-17d3-41e6-b9c9-fc8c39442...@eisentraut.org --- src/backend/libpq/auth-scram.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 03ddddc3c27..56df870e9ef 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -608,13 +608,17 @@ parse_scram_secret(const char *secret, int *iterations, * SCRAM-SHA-256$<iterations>:<salt>$<storedkey>:<serverkey> */ v = pstrdup(secret); - if ((scheme_str = strsep(&v, "$")) == NULL) + scheme_str = strsep(&v, "$"); + if (v == NULL) goto invalid_secret; - if ((iterations_str = strsep(&v, ":")) == NULL) + iterations_str = strsep(&v, ":"); + if (v == NULL) goto invalid_secret; - if ((salt_str = strsep(&v, "$")) == NULL) + salt_str = strsep(&v, "$"); + if (v == NULL) goto invalid_secret; - if ((storedkey_str = strsep(&v, ":")) == NULL) + storedkey_str = strsep(&v, ":"); + if (v == NULL) goto invalid_secret; serverkey_str = v; base-commit: 7cdfeee320e72162b62dddddee638e713c2b8680 -- 2.47.0
