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 0c9fec0e676fc1140a2b4338c8df0a904b551d3f..1795b8cde4256f301440335ab24906d4fd72d634 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 94fb2cf11c6bdbf1cd82cdfaf1019e0401225c63..e20fbbb625a99894360804c6aecc06c41a773b19 100644 (file)
--- a/contrib/pg_standby/pg_standby.c
+++ b/contrib/pg_standby/pg_standby.c
@@ -338,7 +338,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 e58ca0480e9ef00bbc06b9eebd5aa1a0887cb36f..32e208c7c72873031eb84a9e6bc8c23c82263a55 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5674,7 +5674,7 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
 
                recordRestorePointData = (xl_restore_point *) XLogRecGetData(record);
                recordXtime = recordRestorePointData->rp_time;
-               strncpy(recordRPName, recordRestorePointData->rp_name, MAXFNAMELEN);
+               strlcpy(recordRPName, recordRestorePointData->rp_name, MAXFNAMELEN);
        }
        else
                return false;
@@ -5769,7 +5769,7 @@ recoveryStopsHere(XLogRecord *record, bool *includeThis)
                }
                else
                {
-                       strncpy(recoveryStopName, recordRPName, MAXFNAMELEN);
+                       strlcpy(recoveryStopName, recordRPName, MAXFNAMELEN);
 
                        ereport(LOG,
                                (errmsg("recovery stopping at restore point \"%s\", time %s",
@@ -6163,7 +6163,7 @@ StartupXLOG(void)
         * see them
         */
        XLogCtl->RecoveryTargetTLI = recoveryTargetTLI;
-       strncpy(XLogCtl->archiveCleanupCommand,
+       strlcpy(XLogCtl->archiveCleanupCommand,
                        archiveCleanupCommand ? archiveCleanupCommand : "",
                        sizeof(XLogCtl->archiveCleanupCommand));
 
@@ -8403,7 +8403,7 @@ XLogRestorePoint(const char *rpName)
        xl_restore_point xlrec;
 
        xlrec.rp_time = GetCurrentTimestamp();
-       strncpy(xlrec.rp_name, rpName, MAXFNAMELEN);
+       strlcpy(xlrec.rp_name, rpName, MAXFNAMELEN);
 
        rdata.buffer = InvalidBuffer;
        rdata.data = (char *) &xlrec;

diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index be1663cd8810e5ae26b07cf6ef3e7c592180513a..a8cbb2015729b96ba24c56bed2f2238a2aec46e8 100644 (file)
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -255,7 +255,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 589f4cd9a1ea71e08ad7f8b31be921e062760b63..05e1730c881bc2c89ccc8a8101b44236143b7503 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 */
@@ -4161,6 +4161,7 @@ ConvertTimeZoneAbbrevs(TimeZoneAbbrevTable *tbl,
        tbl->numabbrevs = n;
        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 / MINS_PER_HOUR);

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 8e94369095d56dd41e341df62fc3c8cafaf74624..64a2604f5b9de334a4a9bbe4389845a89c76b130 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3242,7 +3242,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/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 3ed6d17dd3bbad8b1421823e6e785e161cb343f5..6eb4e1b8dad1cfc02995c7287cae621a31c7947e 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -508,7 +508,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
        FILE       *file = NULL;
 
        if (PQgetisnull(res, rownum, 0))
-               strcpy(current_path, basedir);
+               strlcpy(current_path, basedir, sizeof(current_path));
        else
        {
                if (PQgetlength(res, rownum, 1) >= MAXPGPATH)
@@ -517,7 +517,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
                                        progname, PQgetvalue(res, rownum, 1));
                        disconnect_and_exit(1);
                }
-               strcpy(current_path, PQgetvalue(res, rownum, 1));
+               strlcpy(current_path, PQgetvalue(res, rownum, 1), sizeof(current_path));
        }
 
        /*

diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index 2fdd56749284da084398924e12f6183d4db44788..40bdd8d6d9d20917e961a4f8555708e1adfbb088 100644 (file)
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -1314,7 +1314,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 77c4d5ac571bf60af22b70826825c736abfdcf11..6f8ac69733cc776c3ec96c3c263df6644da254a0 100644 (file)
--- a/src/interfaces/libpq/fe-protocol2.c
+++ b/src/interfaces/libpq/fe-protocol2.c
@@ -500,7 +500,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 45a84d8e553dc5ffa7183241222950804cbfcc8f..18b7483db79c945186d1604bf3417a6e1839a617 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 d153d352f13ef1a6b7d8359676f87ed8ced79512..cc32077badd94fadc901e0528d63d1ad0e2c7fc3 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 502299a52e2772c9dc84e4791484742129d760c2..37253713d6e7e13e8476f4831c260cc3b8267087 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1242,7 +1242,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)
        {
                /*
@@ -1297,7 +1297,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);
        }
@@ -1325,7 +1325,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 4619a273e24baa08d6ed24748dd4d09194e33a69..324f2369051c7fbbdd4c48b4c9665b6dae31b34e 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,