From 5a8348d63f9d5265ce9d33bb631f49c186a72f9d Mon Sep 17 00:00:00 2001 From: Sehrope Sarkuni Date: Sun, 24 May 2026 10:56:12 -0400 Subject: [PATCH 1/6] Fix int overflow when parsing SCRAM iteration count parse_scram_secret() in the backend and read_server_first_message() in libpq both parsed the iteration count with strtol() into a long and then assigned the result to int *iterations. The errno check following strtol() catches ERANGE (overflow of long), but on platforms where sizeof(long) > sizeof(int) a digit string in (INT_MAX, LONG_MAX] is parsed successfully, errno is not set, and the assignment to int silently truncates the value. Practically the truncation does not cause SCRAM authentication to succeed against a verifier the client did not have the password for, since the client and server arrive at different SaltedPasswords. The authentication just fails the wrong way, with the client either running far fewer PBKDF2 iterations than the server expected or performing a different amount of work entirely. The backend variant also silently accepted zero and negative iteration counts because no range check was present at all. Add scram_parse_iterations() in src/common/scram-common.c that rejects trailing garbage, ERANGE (overflow of long), values that would narrow incorrectly to int, and values below 1 as required by RFC 5802. Replace both ad-hoc parses with a call to the helper. --- src/backend/libpq/auth-scram.c | 5 +-- src/common/scram-common.c | 53 ++++++++++++++++++++++++++++ src/include/common/scram-common.h | 2 ++ src/interfaces/libpq/fe-auth-scram.c | 4 +-- 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 4bac15fc5c1..43205b41155 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -600,7 +600,6 @@ parse_scram_secret(const char *secret, int *iterations, char **salt, uint8 *stored_key, uint8 *server_key) { char *v; - char *p; char *scheme_str; char *salt_str; char *iterations_str; @@ -637,9 +636,7 @@ parse_scram_secret(const char *secret, int *iterations, *hash_type = PG_SHA256; *key_length = SCRAM_SHA_256_KEY_LEN; - errno = 0; - *iterations = strtol(iterations_str, &p, 10); - if (*p || errno != 0) + if (!scram_parse_iterations(iterations_str, iterations)) goto invalid_secret; /* diff --git a/src/common/scram-common.c b/src/common/scram-common.c index 259fa5554b6..0644771b5a6 100644 --- a/src/common/scram-common.c +++ b/src/common/scram-common.c @@ -19,6 +19,8 @@ #include "postgres_fe.h" #endif +#include + #include "common/base64.h" #include "common/hmac.h" #include "common/scram-common.h" @@ -327,3 +329,54 @@ scram_build_secret(pg_cryptohash_type hash_type, int key_length, return result; } + +/* + * Parse a SCRAM iteration count from a null-terminated string. + * + * On success, stores the parsed value in *iterations and returns true. + * On failure, *iterations is unchanged and false is returned. + * + * The accepted input is a non-empty sequence of ASCII digits ('0'-'9'). + * Leading whitespace, a leading '+' or '-' sign, and any trailing + * non-digit characters are rejected, even though strtol() would + * otherwise accept them; the SCRAM verifier and server-first-message + * formats both define the iteration count as a sequence of digits with + * no surrounding decoration, so anything else is malformed. + * + * Beyond digit shape we also reject ERANGE (overflow of long), values + * that would silently truncate when narrowed to int on platforms where + * sizeof(long) is greater than sizeof(int), and any value below 1 + * (RFC 5802 requires a positive iteration count). + */ +bool +scram_parse_iterations(const char *str, int *iterations) +{ + const char *p; + char *endptr; + long val; + + /* Require a non-empty, pure-digit string. */ + if (*str == '\0') + return false; + for (p = str; *p != '\0'; p++) + { + if (*p < '0' || *p > '9') + return false; + } + + errno = 0; + val = strtol(str, &endptr, 10); + + /* + * The digit-only pre-scan guarantees strtol() consumes the entire + * string, so *endptr is always '\0' here. ERANGE still has to be + * checked because strtol() reports long overflow that way; the + * int-range and below-1 checks catch inputs that are valid as longs + * but unacceptable as iteration counts. + */ + Assert(*endptr == '\0'); + if (errno != 0 || val <= 0 || val > INT_MAX) + return false; + *iterations = (int) val; + return true; +} diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h index 52545561bd3..27f42fdef02 100644 --- a/src/include/common/scram-common.h +++ b/src/include/common/scram-common.h @@ -67,4 +67,6 @@ extern char *scram_build_secret(pg_cryptohash_type hash_type, int key_length, const uint8 *salt, int saltlen, int iterations, const char *password, const char **errstr); +extern bool scram_parse_iterations(const char *str, int *iterations); + #endif /* SCRAM_COMMON_H */ diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c index 08c7c666946..ac7ce3c304b 100644 --- a/src/interfaces/libpq/fe-auth-scram.c +++ b/src/interfaces/libpq/fe-auth-scram.c @@ -608,7 +608,6 @@ read_server_first_message(fe_scram_state *state, char *input) { PGconn *conn = state->conn; char *iterations_str; - char *endptr; char *encoded_salt; char *nonce; int decoded_salt_len; @@ -673,8 +672,7 @@ read_server_first_message(fe_scram_state *state, char *input) /* read_attr_value() has appended an error string */ return false; } - state->iterations = strtol(iterations_str, &endptr, 10); - if (*endptr != '\0' || state->iterations < 1) + if (!scram_parse_iterations(iterations_str, &state->iterations)) { libpq_append_conn_error(conn, "malformed SCRAM message (invalid iteration count)"); return false; -- 2.43.0