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 a3f40fbe61aa3da876414da8e6a640f47688b193..b3fbd6ee37004a2dea88dea5a1c68886cea529e8 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 591406ba1015b7a7e48cc789cbb986216e96ed5f..93ee0709801a3edc2772cf8ed7c4dde3c3880e63 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4529,7 +4529,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;
@@ -4624,7 +4624,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",
@@ -4957,7 +4957,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));
 
@@ -7685,7 +7685,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 4e67780f2fac26777a98cef257f52e49b2b4b974..45bed12c59a060f52bbfacf36197539d8f0a9d78 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 7834c9325102fd036aea699098232f610b1874fc..6979e921324853a0bb0fb71e60a255d98cbb60a8 100644 (file)
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -91,10 +91,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
@@ -109,7 +109,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 */
@@ -189,7 +189,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 */
@@ -4213,6 +4213,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 d3a8c17ee228983a9ddcceedf77170f73f73d9fd..24e94326a92a7bf1b1b67dc7244f93d0c96c21b5 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 07afef79656846d4a9b01cdd5b50eb362172c81a..b1bcf698eb66ea92e07cb3b016308b34a8a3e38e 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -903,9 +903,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
@@ -1432,7 +1432,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,
@@ -1544,7 +1544,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/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index 5abb74f14e5cf289b6b105e2b228ad75947721cf..7d7074eaae92ff8321b1be56210776473c8f3328 100644 (file)
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -1315,7 +1315,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 f05845cbb176115762bf3b829bb7b11ded6eb8e8..3f4908fcefaf6ed77b2cb5d8c5d8690d80e5336e 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 b6316803dd41d7cee439fcf5ae55c82f15a28b97..797679f96f6b52a4f212a213359c2de2bd375457 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 01203c056cc068a30628908537456b08a7041c25..8ce7c618ffcddca46d023e2caf724e6e07ff6edb 100644 (file)
--- a/src/port/exec.c
+++ b/src/port/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/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index b632326e08df4f75a31cd34e8f76c715d3fd90f4..5eddb36e9e85536da73e6d5d917dbe73aa55aafd 100644 (file)
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -1233,7 +1233,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)
        {
                /*
@@ -1288,7 +1288,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);
        }
@@ -1316,7 +1316,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 1130de9a961064f79e0f5ce2fb1a5d602f1ae0ef..640410d3876a0c931674d5a3aee8a7423bd69733 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 (;;)