Prevent potential overruns of fixed-size buffers.

Coverity identified a number of places in which it couldn't prove that a
string being copied into a fixed-size buffer would fit.  We believe that
most, perhaps all of these are in fact safe, or are copying data that is
coming from a trusted source so that any overrun is not really a security
issue.  Nonetheless it seems prudent to forestall any risk by using
strlcpy() and similar functions.

Fixes by Peter Eisentraut and Jozef Mlich based on Coverity reports.

In addition, fix a potential null-pointer-dereference crash in
contrib/chkpass.  The crypt(3) function is defined to return NULL on
failure, but chkpass.c didn't check for that before using the result.
The main practical case in which this could be an issue is if libc is
configured to refuse to execute unapproved hashing algorithms (e.g.,
"FIPS mode").  This ideally should've been a separate commit, but
since it touches code adjacent to one of the buffer overrun changes,
I included it in this commit to avoid last-minute merge issues.
This issue was reported by Honza Horak.

Security: CVE-2014-0065 for buffer overruns, CVE-2014-0066 for crypt()

diff --git a/contrib/chkpass/chkpass.c b/contrib/chkpass/chkpass.c
index 56a998ec839cc5d2469fe99e6c88b60e8b7a695e..b54aacca52c3ac200e7d64c8292b0f67299f5e38 100644 (file)
--- a/contrib/chkpass/chkpass.c
+++ b/contrib/chkpass/chkpass.c
@@ -70,6 +70,7 @@ chkpass_in(PG_FUNCTION_ARGS)
        char       *str = PG_GETARG_CSTRING(0);
        chkpass    *result;
        char            mysalt[4];
+       char       *crypt_output;
        static char salt_chars[] =
        "./0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
 
@@ -92,7 +93,15 @@ chkpass_in(PG_FUNCTION_ARGS)
        mysalt[1] = salt_chars[random() & 0x3f];
        mysalt[2] = 0;                          /* technically the terminator is not necessary
                                                                 * but I like to play safe */
-       strcpy(result->password, crypt(str, mysalt));
+
+       crypt_output = crypt(str, mysalt);
+       if (crypt_output == NULL)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("crypt() failed")));
+
+       strlcpy(result->password, crypt_output, sizeof(result->password));
+
        PG_RETURN_POINTER(result);
 }
 
@@ -141,9 +150,16 @@ chkpass_eq(PG_FUNCTION_ARGS)
        chkpass    *a1 = (chkpass *) PG_GETARG_POINTER(0);
        text       *a2 = PG_GETARG_TEXT_PP(1);
        char            str[9];
+       char       *crypt_output;
 
        text_to_cstring_buffer(a2, str, sizeof(str));
-       PG_RETURN_BOOL(strcmp(a1->password, crypt(str, a1->password)) == 0);
+       crypt_output = crypt(str, a1->password);
+       if (crypt_output == NULL)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("crypt() failed")));
+
+       PG_RETURN_BOOL(strcmp(a1->password, crypt_output) == 0);
 }
 
 PG_FUNCTION_INFO_V1(chkpass_ne);
@@ -153,7 +169,14 @@ chkpass_ne(PG_FUNCTION_ARGS)
        chkpass    *a1 = (chkpass *) PG_GETARG_POINTER(0);
        text       *a2 = PG_GETARG_TEXT_PP(1);
        char            str[9];
+       char       *crypt_output;
 
        text_to_cstring_buffer(a2, str, sizeof(str));
-       PG_RETURN_BOOL(strcmp(a1->password, crypt(str, a1->password)) != 0);
+       crypt_output = crypt(str, a1->password);
+       if (crypt_output == NULL)
+               ereport(ERROR,
+                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                errmsg("crypt() failed")));
+
+       PG_RETURN_BOOL(strcmp(a1->password, crypt_output) != 0);
 }

diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c
index 70680e762d26c8c0488a1db27527c7377c90473c..adfbad5f388701b3c74cbd20152ac19652f6c777 100644 (file)
--- a/contrib/pg_standby/pg_standby.c
+++ b/contrib/pg_standby/pg_standby.c
@@ -337,7 +337,7 @@ SetWALFileNameForCleanup(void)
                if (strcmp(restartWALFileName, nextWALFileName) > 0)
                        return false;
 
-               strcpy(exclusiveCleanupFileName, restartWALFileName);
+               strlcpy(exclusiveCleanupFileName, restartWALFileName, sizeof(exclusiveCleanupFileName));
                return true;
        }
 

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7424201c43e4245a054b1ef215e5a0bfda6beb42..20bcdcd401b1f7e7f979e53784c45b027be8c8ef 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5860,7 +5860,7 @@ StartupXLOG(void)
         * see them
         */
        XLogCtl->RecoveryTargetTLI = recoveryTargetTLI;
-       strncpy(XLogCtl->archiveCleanupCommand,
+       strlcpy(XLogCtl->archiveCleanupCommand,
                        archiveCleanupCommand ? archiveCleanupCommand : "",
                        sizeof(XLogCtl->archiveCleanupCommand));
 

diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index cbfd3860742c48849f0f0ecb2cc74674373ae8d1..561ea399d73f5bb3e6a0e36f1ad0da9ce125ccd2 100644 (file)
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -181,7 +181,7 @@ NIAddSpell(IspellDict *Conf, const char *word, const char *flag)
        }
        Conf->Spell[Conf->nspell] = (SPELL *) tmpalloc(SPELLHDRSZ + strlen(word) + 1);
        strcpy(Conf->Spell[Conf->nspell]->word, word);
-       strncpy(Conf->Spell[Conf->nspell]->p.flag, flag, MAXFLAGLEN);
+       strlcpy(Conf->Spell[Conf->nspell]->p.flag, flag, MAXFLAGLEN);
        Conf->nspell++;
 }
 

diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 95d4556c734fcf36dc2a9aa9b155cd7b7ce16afb..fb9a95a1bd59ffc61768d48981606b00f9387589 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -89,10 +89,10 @@ char           *days[] = {"Sunday", "Monday", "Tuesday", "Wednesday",
  * Note that this table must be strictly alphabetically ordered to allow an
  * O(ln(N)) search algorithm to be used.
  *
- * The text field is NOT guaranteed to be NULL-terminated.
+ * The token field is NOT guaranteed to be NULL-terminated.
  *
- * To keep this table reasonably small, we divide the lexval for TZ and DTZ
- * entries by 15 (so they are on 15 minute boundaries) and truncate the text
+ * To keep this table reasonably small, we divide the value for TZ and DTZ
+ * entries by 15 (so they are on 15 minute boundaries) and truncate the token
  * field at TOKMAXLEN characters.
  * Formerly, we divided by 10 rather than 15 but there are a few time zones
  * which are 30 or 45 minutes away from an even hour, most are on an hour
@@ -107,7 +107,7 @@ static datetkn *timezonetktbl = NULL;
 static int     sztimezonetktbl = 0;
 
 static const datetkn datetktbl[] = {
-/*     text, token, lexval */
+       /* token, type, value */
        {EARLY, RESERV, DTK_EARLY}, /* "-infinity" reserved for "early time" */
        {DA_D, ADBC, AD},                       /* "ad" for years > 0 */
        {"allballs", RESERV, DTK_ZULU},         /* 00:00:00 */
@@ -187,7 +187,7 @@ static const datetkn datetktbl[] = {
 static int     szdatetktbl = sizeof datetktbl / sizeof datetktbl[0];
 
 static datetkn deltatktbl[] = {
-       /* text, token, lexval */
+       /* token, type, value */
        {"@", IGNORE_DTF, 0},           /* postgres relative prefix */
        {DAGO, AGO, 0},                         /* "ago" indicates negative time offset */
        {"c", UNITS, DTK_CENTURY},      /* "century" relative */
@@ -4152,6 +4152,7 @@ InstallTimeZoneAbbrevs(tzEntry *abbrevs, int n)
                                                                                        n * sizeof(datetkn));
        for (i = 0; i < n; i++)
        {
+               /* do NOT use strlcpy here; token field need not be null-terminated */
                strncpy(newtbl[i].token, abbrevs[i].abbrev, TOKMAXLEN);
                newtbl[i].type = abbrevs[i].is_dst ? DTZ : TZ;
                TOVAL(&newtbl[i], abbrevs[i].offset / 60);

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index f3e1d90dc41d5050e980238a4147e5b65aab3542..901aed967877ed32e05463c63f5f7f2c58f5832f 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3130,7 +3130,7 @@ main(int argc, char *argv[])
                fprintf(stderr, "%s", authwarning);
 
        /* Get directory specification used to start this executable */
-       strcpy(bin_dir, argv[0]);
+       strlcpy(bin_dir, argv[0], sizeof(bin_dir));
        get_parent_directory(bin_dir);
 
        printf(_("\nSuccess. You can now start the database server using:\n\n"

diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index c7d297a12a37491302c00b0e2e57474e7c026133..bc16cc5b7fcd0503ae571071ef7093f41a384132 100644 (file)
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -1312,7 +1312,7 @@ parse_include(void)
                yytext[i] = '\0';
                memmove(yytext, yytext+1, strlen(yytext));
        
-               strncpy(inc_file, yytext, sizeof(inc_file));
+               strlcpy(inc_file, yytext, sizeof(inc_file));
                yyin = fopen(inc_file, "r");
                if (!yyin)
                {

diff --git a/src/interfaces/libpq/fe-protocol2.c b/src/interfaces/libpq/fe-protocol2.c
index 0b3bd3b9aa958e6a5a01760997bb291c7c123b13..66124641ede4aac4575f20c052af84a4fc19df2d 100644 (file)
--- a/src/interfaces/libpq/fe-protocol2.c
+++ b/src/interfaces/libpq/fe-protocol2.c
@@ -440,7 +440,7 @@ pqParseInput2(PGconn *conn)
                                                if (!conn->result)
                                                        return;
                                        }
-                                       strncpy(conn->result->cmdStatus, conn->workBuffer.data,
+                                       strlcpy(conn->result->cmdStatus, conn->workBuffer.data,
                                                        CMDSTATUS_LEN);
                                        checkXactStatus(conn, conn->workBuffer.data);
                                        conn->asyncStatus = PGASYNC_READY;

diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 28318f006977f370bb9e545cba4f6bdd3afbd5ee..0c484824e68e871e19a6a9723d3f453cc62cad6f 100644 (file)
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -206,7 +206,7 @@ pqParseInput3(PGconn *conn)
                                                if (!conn->result)
                                                        return;
                                        }
-                                       strncpy(conn->result->cmdStatus, conn->workBuffer.data,
+                                       strlcpy(conn->result->cmdStatus, conn->workBuffer.data,
                                                        CMDSTATUS_LEN);
                                        conn->asyncStatus = PGASYNC_READY;
                                        break;

diff --git a/src/port/exec.c b/src/port/exec.c
index 5dd5963f2c5a11696749347975b6161e2531a922..1ef2b11c034e39bc3905e122e9a3f3abdbbd0788 100644 (file)
--- a/src/port/exec.c
+++ b/src/port/exec.c
@@ -66,7 +66,7 @@ validate_exec(const char *path)
        if (strlen(path) >= strlen(".exe") &&
                pg_strcasecmp(path + strlen(path) - strlen(".exe"), ".exe") != 0)
        {
-               strcpy(path_exe, path);
+               strlcpy(path_exe, path, sizeof(path_exe) - 4);
                strcat(path_exe, ".exe");
                path = path_exe;
        }
@@ -275,7 +275,7 @@ resolve_symlinks(char *path)
        }
 
        /* must copy final component out of 'path' temporarily */
-       strcpy(link_buf, fname);
+       strlcpy(link_buf, fname, sizeof(link_buf));
 
        if (!getcwd(path, MAXPGPATH))
        {

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 43edf1ea4b7062eca401a5c657a76fe011d0eebe..4509f213a78f143a89977caca4731ff92e117506 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1237,7 +1237,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
         */
        platform_expectfile = get_expectfile(testname, resultsfile);
 
-       strcpy(expectfile, default_expectfile);
+       strlcpy(expectfile, default_expectfile, sizeof(expectfile));
        if (platform_expectfile)
        {
                /*
@@ -1292,7 +1292,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
                {
                        /* This diff was a better match than the last one */
                        best_line_count = l;
-                       strcpy(best_expect_file, alt_expectfile);
+                       strlcpy(best_expect_file, alt_expectfile, sizeof(best_expect_file));
                }
                free(alt_expectfile);
        }
@@ -1320,7 +1320,7 @@ results_differ(const char *testname, const char *resultsfile, const char *defaul
                {
                        /* This diff was a better match than the last one */
                        best_line_count = l;
-                       strcpy(best_expect_file, default_expectfile);
+                       strlcpy(best_expect_file, default_expectfile, sizeof(best_expect_file));
                }
        }
 

diff --git a/src/timezone/pgtz.c b/src/timezone/pgtz.c
index 13e3bf194400dacbea4f964e2d62805997ba93a5..e286c1178c20352d11d456d02c2e68cdd2e89783 100644 (file)
--- a/src/timezone/pgtz.c
+++ b/src/timezone/pgtz.c
@@ -94,7 +94,7 @@ pg_open_tzfile(const char *name, char *canonname)
         * Loop to split the given name into directory levels; for each level,
         * search using scan_directory_ci().
         */
-       strcpy(fullname, pg_TZDIR());
+       strlcpy(fullname, pg_TZDIR(), sizeof(fullname));
        orignamelen = fullnamelen = strlen(fullname);
        fname = name;
        for (;;)
@@ -428,7 +428,7 @@ identify_system_timezone(void)
        }
 
        /* Search for the best-matching timezone file */
-       strcpy(tmptzdir, pg_TZDIR());
+       strlcpy(tmptzdir, pg_TZDIR(), sizeof(tmptzdir));
        bestscore = -1;
        resultbuf[0] = '\0';
        scan_available_timezones(tmptzdir, tmptzdir + strlen(tmptzdir) + 1,