aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2018-09-08 18:20:36 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2018-09-08 18:20:36 -0400
commit930b785d40cf53d679c72ffc2c34a63d412bee5b (patch)
tree16a1c2346e235ff05ae77a273ba56e3fe680df19
parent3985b75dca6d1101cc4cb6e78456dc6c5f72fcac (diff)
downloadpostgresql-930b785d40cf53d679c72ffc2c34a63d412bee5b.tar.gz
postgresql-930b785d40cf53d679c72ffc2c34a63d412bee5b.zip
Minor cleanup/future-proofing for pg_saslprep().
Ensure that pg_saslprep() initializes its output argument to NULL in all failure paths, and then remove the redundant initialization that some (not all) of its callers did. This does not fix any live bug, but it reduces the odds of future bugs of omission. Also add a comment about why the existing failure-path coding is adequate. Back-patch so as to keep the function's API consistent across branches, again to forestall future bug introduction. Patch by me, reviewed by Michael Paquier Discussion: https://postgr.es/m/16558.1536407783@sss.pgh.pa.us
-rw-r--r--src/backend/libpq/auth-scram.c4
-rw-r--r--src/common/saslprep.c11
-rw-r--r--src/interfaces/libpq/fe-auth-scram.c2
3 files changed, 11 insertions, 6 deletions
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 7cd31ebe8e2..d69d7dde06a 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -382,7 +382,7 @@ pg_be_scram_exchange(void *opaq, char *input, int inputlen,
char *
pg_be_scram_build_verifier(const char *password)
{
- char *prep_password = NULL;
+ char *prep_password;
pg_saslprep_rc rc;
char saltbuf[SCRAM_DEFAULT_SALT_LEN];
char *result;
@@ -428,7 +428,7 @@ scram_verify_plain_password(const char *username, const char *password,
uint8 stored_key[SCRAM_KEY_LEN];
uint8 server_key[SCRAM_KEY_LEN];
uint8 computed_key[SCRAM_KEY_LEN];
- char *prep_password = NULL;
+ char *prep_password;
pg_saslprep_rc rc;
if (!parse_scram_verifier(verifier, &iterations, &encoded_salt,
diff --git a/src/common/saslprep.c b/src/common/saslprep.c
index 0a3585850b8..32efc31591e 100644
--- a/src/common/saslprep.c
+++ b/src/common/saslprep.c
@@ -1081,6 +1081,9 @@ pg_saslprep(const char *input, char **output)
unsigned char *p;
pg_wchar *wp;
+ /* Ensure we return *output as NULL on failure */
+ *output = NULL;
+
/* Check that the password isn't stupendously long */
if (strlen(input) > MAX_PASSWORD_LENGTH)
{
@@ -1112,10 +1115,7 @@ pg_saslprep(const char *input, char **output)
*/
input_size = pg_utf8_string_len(input);
if (input_size < 0)
- {
- *output = NULL;
return SASLPREP_INVALID_UTF8;
- }
input_chars = ALLOC((input_size + 1) * sizeof(pg_wchar));
if (!input_chars)
@@ -1246,6 +1246,11 @@ pg_saslprep(const char *input, char **output)
result = ALLOC(result_size + 1);
if (!result)
goto oom;
+
+ /*
+ * There are no error exits below here, so the error exit paths don't need
+ * to worry about possibly freeing "result".
+ */
p = (unsigned char *) result;
for (wp = output_chars; *wp; wp++)
{
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index edfd42df854..7fa7f34c80b 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -621,7 +621,7 @@ verify_server_signature(fe_scram_state *state)
char *
pg_fe_scram_build_verifier(const char *password)
{
- char *prep_password = NULL;
+ char *prep_password;
pg_saslprep_rc rc;
char saltbuf[SCRAM_DEFAULT_SALT_LEN];
char *result;