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

diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 7cd31ebe8e201d95e0dd8068e3a829273651ccf4..d69d7dde06a6ac136fd36c92b8e5fb07ab8efac8 100644 (file)
--- 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 0a3585850b852457d3e2676b71d017743e37c0c4..32efc31591e99c7bc883e92c70b308dea92949d2 100644 (file)
--- 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 edfd42df8540161603d4edf7c2037c50614a7152..7fa7f34c80be2f17d10f4556b8f04fd720409dfd 100644 (file)
--- 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;