From 09ec55b933091cb5b0af99978718cb3d289c71b6 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 17 Jun 2019 21:48:17 +0900 Subject: [PATCH] Fix buffer overflow when parsing SCRAM verifiers in backend Any authenticated user can overflow a stack-based buffer by changing the user's own password to a purpose-crafted value. This often suffices to execute arbitrary code as the PostgreSQL operating system account. This fix is contributed by multiple folks, based on an initial analysis from Tom Lane. This issue has been introduced by 68e61ee, so it was possible to make use of it at authentication time. It became more easily to trigger after ccae190 which has made the SCRAM parsing more strict when changing a password, in the case where the client passes down a verifier already hashed using SCRAM. Back-patch to v10 where SCRAM has been introduced. Reported-by: Alexander Lakhin Author: Jonathan Katz, Heikki Linnakangas, Michael Paquier Security: CVE-2019-10164 Backpatch-through: 10 --- src/backend/libpq/auth-scram.c | 35 ++++++++++++++++++++------ src/test/regress/expected/password.out | 23 +++++++++++++++++ src/test/regress/sql/password.sql | 18 +++++++++++++ 3 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c index 45811e155c..6b60abe1dd 100644 --- a/src/backend/libpq/auth-scram.c +++ b/src/backend/libpq/auth-scram.c @@ -542,6 +542,12 @@ scram_verify_plain_password(const char *username, const char *password, /* * Parse and validate format of given SCRAM verifier. * + * On success, the iteration count, salt, stored key, and server key are + * extracted from the verifier, and returned to the caller. For 'stored_key' + * and 'server_key', the caller must pass pre-allocated buffers of size + * SCRAM_KEY_LEN. Salt is returned as a base64-encoded, null-terminated + * string. The buffer for the salt is palloc'd by this function. + * * Returns true if the SCRAM verifier has been parsed, and false otherwise. */ bool @@ -557,6 +563,8 @@ parse_scram_verifier(const char *verifier, int *iterations, char **salt, char *serverkey_str; int decoded_len; char *decoded_salt_buf; + char *decoded_stored_buf; + char *decoded_server_buf; /* * The verifier is of form: @@ -589,7 +597,8 @@ parse_scram_verifier(const char *verifier, int *iterations, char **salt, * although we return the encoded version to the caller. */ decoded_salt_buf = palloc(pg_b64_dec_len(strlen(salt_str))); - decoded_len = pg_b64_decode(salt_str, strlen(salt_str), decoded_salt_buf); + decoded_len = pg_b64_decode(salt_str, strlen(salt_str), + decoded_salt_buf); if (decoded_len < 0) goto invalid_verifier; *salt = pstrdup(salt_str); @@ -597,28 +606,38 @@ parse_scram_verifier(const char *verifier, int *iterations, char **salt, /* * Decode StoredKey and ServerKey. */ - if (pg_b64_dec_len(strlen(storedkey_str) != SCRAM_KEY_LEN)) - goto invalid_verifier; + decoded_stored_buf = palloc(pg_b64_dec_len(strlen(storedkey_str))); decoded_len = pg_b64_decode(storedkey_str, strlen(storedkey_str), - (char *) stored_key); + decoded_stored_buf); if (decoded_len != SCRAM_KEY_LEN) goto invalid_verifier; + memcpy(stored_key, decoded_stored_buf, SCRAM_KEY_LEN); - if (pg_b64_dec_len(strlen(serverkey_str) != SCRAM_KEY_LEN)) - goto invalid_verifier; + decoded_server_buf = palloc(pg_b64_dec_len(strlen(serverkey_str))); decoded_len = pg_b64_decode(serverkey_str, strlen(serverkey_str), - (char *) server_key); + decoded_server_buf); if (decoded_len != SCRAM_KEY_LEN) goto invalid_verifier; + memcpy(server_key, decoded_server_buf, SCRAM_KEY_LEN); return true; invalid_verifier: - pfree(v); *salt = NULL; return false; } +/* + * Generate plausible SCRAM verifier parameters for mock authentication. + * + * In a normal authentication, these are extracted from the verifier + * stored in the server. This function generates values that look + * realistic, for when there is no stored verifier. + * + * Like in parse_scram_verifier(), for 'stored_key' and 'server_key', the + * caller must pass pre-allocated buffers of size SCRAM_KEY_LEN, and + * the buffer for the salt is palloc'd by this function. + */ static void mock_scram_verifier(const char *username, int *iterations, char **salt, uint8 *stored_key, uint8 *server_key) diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out index 971e290a32..f1ae34f15f 100644 --- a/src/test/regress/expected/password.out +++ b/src/test/regress/expected/password.out @@ -100,6 +100,26 @@ SELECT rolpassword FROM pg_authid WHERE rolname='regress_passwd_empty'; (1 row) +-- Test with invalid stored and server keys. +-- +-- The first is valid, to act as a control. The others have too long +-- stored/server keys. They will be re-hashed. +CREATE ROLE regress_passwd_sha_len0 PASSWORD 'SCRAM-SHA-256$4096:A6xHKoH/494E941doaPOYg==$Ky+A30sewHIH3VHQLRN9vYsuzlgNyGNKCh37dy96Rqw=:COPdlNiIkrsacU5QoxydEuOH6e/KfiipeETb/bPw8ZI='; +CREATE ROLE regress_passwd_sha_len1 PASSWORD 'SCRAM-SHA-256$4096:A6xHKoH/494E941doaPOYg==$Ky+A30sewHIH3VHQLRN9vYsuzlgNyGNKCh37dy96RqwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=:COPdlNiIkrsacU5QoxydEuOH6e/KfiipeETb/bPw8ZI='; +CREATE ROLE regress_passwd_sha_len2 PASSWORD 'SCRAM-SHA-256$4096:A6xHKoH/494E941doaPOYg==$Ky+A30sewHIH3VHQLRN9vYsuzlgNyGNKCh37dy96Rqw=:COPdlNiIkrsacU5QoxydEuOH6e/KfiipeETb/bPw8ZIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='; +-- Check that the invalid verifiers were re-hashed. A re-hashed verifier +-- should not contain the original salt. +SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassword_rehashed + FROM pg_authid + WHERE rolname LIKE 'regress_passwd_sha_len%' + ORDER BY rolname; + rolname | is_rolpassword_rehashed +-------------------------+------------------------- + regress_passwd_sha_len0 | f + regress_passwd_sha_len1 | t + regress_passwd_sha_len2 | t +(3 rows) + DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; @@ -109,6 +129,9 @@ DROP ROLE regress_passwd6; DROP ROLE regress_passwd7; DROP ROLE regress_passwd8; DROP ROLE regress_passwd_empty; +DROP ROLE regress_passwd_sha_len0; +DROP ROLE regress_passwd_sha_len1; +DROP ROLE regress_passwd_sha_len2; -- all entries should have been removed SELECT rolname, rolpassword FROM pg_authid diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql index 89b6d4b278..fad88ba689 100644 --- a/src/test/regress/sql/password.sql +++ b/src/test/regress/sql/password.sql @@ -75,6 +75,21 @@ ALTER ROLE regress_passwd_empty PASSWORD 'md585939a5ce845f1a1b620742e3c659e0a'; ALTER ROLE regress_passwd_empty PASSWORD 'SCRAM-SHA-256$4096:hpFyHTUsSWcR7O9P$LgZFIt6Oqdo27ZFKbZ2nV+vtnYM995pDh9ca6WSi120=:qVV5NeluNfUPkwm7Vqat25RjSPLkGeoZBQs6wVv+um4='; SELECT rolpassword FROM pg_authid WHERE rolname='regress_passwd_empty'; +-- Test with invalid stored and server keys. +-- +-- The first is valid, to act as a control. The others have too long +-- stored/server keys. They will be re-hashed. +CREATE ROLE regress_passwd_sha_len0 PASSWORD 'SCRAM-SHA-256$4096:A6xHKoH/494E941doaPOYg==$Ky+A30sewHIH3VHQLRN9vYsuzlgNyGNKCh37dy96Rqw=:COPdlNiIkrsacU5QoxydEuOH6e/KfiipeETb/bPw8ZI='; +CREATE ROLE regress_passwd_sha_len1 PASSWORD 'SCRAM-SHA-256$4096:A6xHKoH/494E941doaPOYg==$Ky+A30sewHIH3VHQLRN9vYsuzlgNyGNKCh37dy96RqwAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=:COPdlNiIkrsacU5QoxydEuOH6e/KfiipeETb/bPw8ZI='; +CREATE ROLE regress_passwd_sha_len2 PASSWORD 'SCRAM-SHA-256$4096:A6xHKoH/494E941doaPOYg==$Ky+A30sewHIH3VHQLRN9vYsuzlgNyGNKCh37dy96Rqw=:COPdlNiIkrsacU5QoxydEuOH6e/KfiipeETb/bPw8ZIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA='; + +-- Check that the invalid verifiers were re-hashed. A re-hashed verifier +-- should not contain the original salt. +SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassword_rehashed + FROM pg_authid + WHERE rolname LIKE 'regress_passwd_sha_len%' + ORDER BY rolname; + DROP ROLE regress_passwd1; DROP ROLE regress_passwd2; DROP ROLE regress_passwd3; @@ -84,6 +99,9 @@ DROP ROLE regress_passwd6; DROP ROLE regress_passwd7; DROP ROLE regress_passwd8; DROP ROLE regress_passwd_empty; +DROP ROLE regress_passwd_sha_len0; +DROP ROLE regress_passwd_sha_len1; +DROP ROLE regress_passwd_sha_len2; -- all entries should have been removed SELECT rolname, rolpassword -- 2.40.0