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 dc66075f9886d4b14a3b639224833f9cb5b73b45..1795b8cde4256f301440335ab24906d4fd72d634 100644 (file)
--- a/contrib/chkpass/chkpass.c
+++ b/contrib/chkpass/chkpass.c
@@ -94,11 +94,13 @@ chkpass_in(PG_FUNCTION_ARGS)
        mysalt[2] = 0;                          /* technically the terminator is not necessary
                                                                 * but I like to play safe */
 
-       if ((crypt_output = crypt(str, mysalt)) == NULL)
+       crypt_output = crypt(str, mysalt);
+       if (crypt_output == NULL)
                ereport(ERROR,
                                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                                 errmsg("crypt() failed")));
-       strcpy(result->password, crypt_output);
+
+       strlcpy(result->password, crypt_output, sizeof(result->password));
 
        PG_RETURN_POINTER(result);
 }
@@ -148,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);
@@ -160,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 144edd8a0728cf97719a86bbe9b666f6a6ea82ac..8ddd486c89eec4907ef8ce7e9a50590dfbb3a9b6 100644 (file)
--- a/contrib/pg_standby/pg_standby.c
+++ b/contrib/pg_standby/pg_standby.c
@@ -327,7 +327,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 85a0ce90180eae2105535bdbed597b932550b001..76c244c915f29d969f7b6c8109dc3e7572fdaf21 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5844,7 +5844,7 @@ recoveryStopsAfter(XLogRecord *record)
                        recoveryStopAfter = true;
                        recoveryStopXid = InvalidTransactionId;
                        (void) getRecordTimestamp(record, &recoveryStopTime);
-                       strncpy(recoveryStopName, recordRestorePointData->rp_name, MAXFNAMELEN);
+                       strlcpy(recoveryStopName, recordRestorePointData->rp_name, MAXFNAMELEN);
 
                        ereport(LOG,
                                        (errmsg("recovery stopping at restore point \"%s\", time %s",
@@ -6311,7 +6311,7 @@ StartupXLOG(void)
         * Save archive_cleanup_command in shared memory so that other processes
         * can see it.
         */
-       strncpy(XLogCtl->archiveCleanupCommand,
+       strlcpy(XLogCtl->archiveCleanupCommand,
                        archiveCleanupCommand ? archiveCleanupCommand : "",
                        sizeof(XLogCtl->archiveCleanupCommand));
 
@@ -9107,7 +9107,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 1ca64423297d2a88a2dabbf89758a223ac41a6e5..1bc226d3347c939bb58c72eb74dc0e61b257ec15 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 946adfad997976fd823b144f8ca4a4cc31571af9..0d32428e407cf6d6063e6a7ceec3cadbbe6568f0 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -89,10 +89,10 @@ const char *const 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 const 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 */
@@ -4215,6 +4215,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/findtimezone.c b/src/bin/initdb/findtimezone.c
index 0d28185f3cd54baf03f5bbabc546cd843122b40e..bc1f2c2c973d5faa9ed4f6db08dd022b323a5a4c 100644 (file)
--- a/src/bin/initdb/findtimezone.c
+++ b/src/bin/initdb/findtimezone.c
@@ -68,7 +68,7 @@ pg_open_tzfile(const char *name, char *canonname)
        if (canonname)
                strlcpy(canonname, name, TZ_STRLEN_MAX + 1);
 
-       strcpy(fullname, pg_TZDIR());
+       strlcpy(fullname, pg_TZDIR(), sizeof(fullname));
        if (strlen(fullname) + 1 + strlen(name) >= MAXPGPATH)
                return -1;                              /* not gonna fit */
        strcat(fullname, "/");
@@ -375,7 +375,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,

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index b5682d6f51191570dcefcd823007b71442a590d3..3d155e8907c2932cc54f9b83d09005565a3b02e1 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -921,9 +921,9 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
        FILE       *file = NULL;
 
        if (basetablespace)
-               strcpy(current_path, basedir);
+               strlcpy(current_path, basedir, sizeof(current_path));
        else
-               strcpy(current_path, PQgetvalue(res, rownum, 1));
+               strlcpy(current_path, PQgetvalue(res, rownum, 1), sizeof(current_path));
 
        /*
         * Get the COPY data
@@ -1454,7 +1454,7 @@ BaseBackup(void)
                disconnect_and_exit(1);
        }
 
-       strcpy(xlogstart, PQgetvalue(res, 0, 0));
+       strlcpy(xlogstart, PQgetvalue(res, 0, 0), sizeof(xlogstart));
 
        /*
         * 9.3 and later sends the TLI of the starting point. With older servers,
@@ -1565,7 +1565,7 @@ BaseBackup(void)
                                progname);
                disconnect_and_exit(1);
        }
-       strcpy(xlogend, PQgetvalue(res, 0, 0));
+       strlcpy(xlogend, PQgetvalue(res, 0, 0), sizeof(xlogend));
        if (verbose && includewal)
                fprintf(stderr, "transaction log end point: %s\n", xlogend);
        PQclear(res);

diff --git a/src/common/exec.c b/src/common/exec.c
index 4aadfd7fd012f2885740c29b641e2c91025fa57e..037bef2210cc4a8923e30eefc88220db3c2ab9f8 100644 (file)
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -68,7 +68,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;
        }
@@ -277,7 +277,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/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index 3bd63b353d078ef8c40992488bc91bca560ca5d8..744a8d3a022ee67025553653e908bd712cd7b148 100644 (file)
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -1334,7 +1334,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 fe7f8b8d2b34c69a22f0665e71f2650b37d5a7bb..f3fddaa0364835b384e27868feaa054228b8933a 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 e2ba8d651f40daaacba46e6d13dc1425a87b3a8a..47cd7f487f0c22d42348c18ce19c1ba631a5d105 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/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 9bc19eaddc46840614259209e2467e0b3e27a887..3a492449fb97f7e39bbb46f6287f192a6d3626d5 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1221,7 +1221,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)
        {
                /*
@@ -1276,7 +1276,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);
        }
@@ -1304,7 +1304,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 8a3c53f27ac5700f90970c158d373a4efc5dc2aa..3c161df15aff2af61435460b4ef7a10b2434d576 100644 (file)
--- a/src/timezone/pgtz.c
+++ b/src/timezone/pgtz.c
@@ -83,7 +83,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 (;;)