Skip to content

Commit 01272d9

Browse files
nmischnmisch
committed
Check return values of sensitive system library calls.
PostgreSQL already checked the vast majority of these, missing this handful that nearly cannot fail. If putenv() failed with ENOMEM in pg_GSS_recvauth(), authentication would proceed with the wrong keytab file. If strftime() returned zero in cache_locale_time(), using the unspecified buffer contents could lead to information exposure or a crash. Back-patch to 9.0 (all supported versions). Other unchecked calls to these functions, especially those in frontend code, pose negligible security concern. This patch does not address them. Nonetheless, it is always better to check return values whose specification provides for indicating an error. In passing, fix an off-by-one error in strftime_win32()'s invocation of WideCharToMultiByte(). Upon retrieving a value of exactly MAX_L10N_DATA bytes, strftime_win32() would overrun the caller's buffer by one byte. MAX_L10N_DATA is chosen to exceed the length of every possible value, so the vulnerable scenario probably does not arise. Security: CVE-2015-3166
1 parent 82b7393 commit 01272d9

File tree

2 files changed

+48
-33
lines changed

2 files changed

+48
-33
lines changed

‎src/backend/libpq/auth.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,15 +1022,16 @@ pg_GSS_recvauth(Port *port)
10221022
size_t kt_len = strlen(pg_krb_server_keyfile) + 14;
10231023
char *kt_path = malloc(kt_len);
10241024

1025-
if (!kt_path)
1025+
if (!kt_path ||
1026+
snprintf(kt_path, kt_len, "KRB5_KTNAME=%s",
1027+
pg_krb_server_keyfile) != kt_len - 2 ||
1028+
putenv(kt_path) != 0)
10261029
{
10271030
ereport(LOG,
10281031
(errcode(ERRCODE_OUT_OF_MEMORY),
10291032
errmsg("out of memory")));
10301033
return STATUS_ERROR;
10311034
}
1032-
snprintf(kt_path, kt_len, "KRB5_KTNAME=%s", pg_krb_server_keyfile);
1033-
putenv(kt_path);
10341035
}
10351036
}
10361037

‎src/backend/utils/adt/pg_locale.c

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -558,24 +558,34 @@ PGLC_localeconv(void)
558558
* pg_strftime(), which isn't locale-aware and does not need to be replaced.
559559
*/
560560
static size_t
561-
strftime_win32(char *dst, size_t dstlen, const wchar_t *format, const struct tm * tm)
561+
strftime_win32(char *dst, size_t dstlen,
562+
const char *format, const struct tm * tm)
562563
{
563564
size_t len;
565+
wchar_t wformat[8]; /* formats used below need 3 bytes */
564566
wchar_t wbuf[MAX_L10N_DATA];
565567
int encoding;
566568

567569
encoding = GetDatabaseEncoding();
568570

569-
len = wcsftime(wbuf, MAX_L10N_DATA, format, tm);
571+
/* get a wchar_t version of the format string */
572+
len = MultiByteToWideChar(CP_UTF8, 0, format, -1,
573+
wformat, lengthof(wformat));
574+
if (len == 0)
575+
elog(ERROR, "could not convert format string from UTF-8: error code %lu",
576+
GetLastError());
577+
578+
len = wcsftime(wbuf, MAX_L10N_DATA, wformat, tm);
570579
if (len == 0)
571580

572581
/*
573-
* strftime call failed - return 0 with the contents of dst
574-
* unspecified
582+
* strftime failed, possibly because the result would not fit in
583+
* MAX_L10N_DATA. Return 0 with the contents of dst unspecified.
575584
*/
576585
return 0;
577586

578-
len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL, NULL);
587+
len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen - 1,
588+
NULL, NULL);
579589
if (len == 0)
580590
elog(ERROR,
581591
"could not convert string to UTF-8: error code %lu", GetLastError());
@@ -598,9 +608,33 @@ strftime_win32(char *dst, size_t dstlen, const wchar_t *format, const struct tm
598608
}
599609

600610
/* redefine strftime() */
601-
#define strftime(a,b,c,d) strftime_win32(a,b,L##c,d)
611+
#define strftime(a,b,c,d) strftime_win32(a,b,c,d)
602612
#endif /* WIN32 */
603613

614+
/* Subroutine for cache_locale_time(). */
615+
static void
616+
cache_single_time(char **dst, const char *format, const struct tm * tm)
617+
{
618+
char buf[MAX_L10N_DATA];
619+
char *ptr;
620+
621+
/*
622+
* MAX_L10N_DATA is sufficient buffer space for every known locale, and
623+
* POSIX defines no strftime() errors. (Buffer space exhaustion is not an
624+
* error.) An implementation might report errors (e.g. ENOMEM) by
625+
* returning 0 (or, less plausibly, a negative value) and setting errno.
626+
* Report errno just in case the implementation did that, but clear it in
627+
* advance of the call so we don't emit a stale, unrelated errno.
628+
*/
629+
errno = 0;
630+
if (strftime(buf, MAX_L10N_DATA, format, tm) <= 0)
631+
elog(ERROR, "strftime(%s) failed: %m", format);
632+
633+
ptr = MemoryContextStrdup(TopMemoryContext, buf);
634+
if (*dst)
635+
pfree(*dst);
636+
*dst = ptr;
637+
}
604638

605639
/*
606640
* Update the lc_time localization cache variables if needed.
@@ -611,8 +645,6 @@ cache_locale_time(void)
611645
char *save_lc_time;
612646
time_t timenow;
613647
struct tm *timeinfo;
614-
char buf[MAX_L10N_DATA];
615-
char *ptr;
616648
int i;
617649

618650
#ifdef WIN32
@@ -659,35 +691,17 @@ cache_locale_time(void)
659691
for (i = 0; i < 7; i++)
660692
{
661693
timeinfo->tm_wday = i;
662-
strftime(buf, MAX_L10N_DATA, "%a", timeinfo);
663-
ptr = MemoryContextStrdup(TopMemoryContext, buf);
664-
if (localized_abbrev_days[i])
665-
pfree(localized_abbrev_days[i]);
666-
localized_abbrev_days[i] = ptr;
667-
668-
strftime(buf, MAX_L10N_DATA, "%A", timeinfo);
669-
ptr = MemoryContextStrdup(TopMemoryContext, buf);
670-
if (localized_full_days[i])
671-
pfree(localized_full_days[i]);
672-
localized_full_days[i] = ptr;
694+
cache_single_time(&localized_abbrev_days[i], "%a", timeinfo);
695+
cache_single_time(&localized_full_days[i], "%A", timeinfo);
673696
}
674697

675698
/* localized months */
676699
for (i = 0; i < 12; i++)
677700
{
678701
timeinfo->tm_mon = i;
679702
timeinfo->tm_mday = 1; /* make sure we don't have invalid date */
680-
strftime(buf, MAX_L10N_DATA, "%b", timeinfo);
681-
ptr = MemoryContextStrdup(TopMemoryContext, buf);
682-
if (localized_abbrev_months[i])
683-
pfree(localized_abbrev_months[i]);
684-
localized_abbrev_months[i] = ptr;
685-
686-
strftime(buf, MAX_L10N_DATA, "%B", timeinfo);
687-
ptr = MemoryContextStrdup(TopMemoryContext, buf);
688-
if (localized_full_months[i])
689-
pfree(localized_full_months[i]);
690-
localized_full_months[i] = ptr;
703+
cache_single_time(&localized_abbrev_months[i], "%b", timeinfo);
704+
cache_single_time(&localized_full_months[i], "%B", timeinfo);
691705
}
692706

693707
/* try to restore internal settings */

0 commit comments

Comments
 (0)