From 6faf7956622c39f21a6b4d0f94d1a43529ad44bf Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 29 Jun 2007 01:51:35 +0000
Subject: [PATCH] Fix a passel of ancient bugs in to_char(), including two
distinct buffer overruns (neither of which seem likely to be exploitable as
security holes, fortunately, since the provoker can't control the data
written). One of these is due to choosing to stomp on the output of a called
function, which is bad news in any case; make it treat the called functions'
results as read-only. Avoid some unnecessary palloc/pfree traffic too; it's
not really helpful to free small temporary objects, and again this is
presuming more than it ought to about the nature of the results of called
functions. Per report from Patrick Welche and additional code-reading by
Imad.
---
src/backend/utils/adt/formatting.c | 150 +++++++++++------------------
1 file changed, 56 insertions(+), 94 deletions(-)
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 6095a1f4fdc..0b54f72e6e0 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -1,7 +1,7 @@
/* -----------------------------------------------------------------------
* formatting.c
*
- * $PostgreSQL: pgsql/src/backend/utils/adt/formatting.c,v 1.129 2007/02/27 23:48:08 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/formatting.c,v 1.130 2007/06/29 01:51:35 tgl Exp $
*
*
* Portions Copyright (c) 1999-2007, PostgreSQL Global Development Group
@@ -74,6 +74,7 @@
#include <unistd.h>
#include <math.h>
#include <float.h>
+#include <limits.h>
#include <locale.h>
#include "utils/builtins.h"
@@ -112,8 +113,8 @@
* More is in float.c
* ----------
*/
-#define MAXFLOATWIDTH 64
-#define MAXDOUBLEWIDTH 128
+#define MAXFLOATWIDTH 60
+#define MAXDOUBLEWIDTH 500
/* ----------
@@ -958,13 +959,12 @@ static char *localize_month(int index);
static char *localize_day_full(int index);
static char *localize_day(int index);
-/*
- * External (defined in oracle_compat.c
- */
#if defined(HAVE_WCSTOMBS) && defined(HAVE_TOWLOWER)
#define USE_WIDE_UPPER_LOWER
+/* externs are in oracle_compat.c */
extern char *wstring_upper(char *str);
extern char *wstring_lower(char *str);
+
static char *localized_str_toupper(char *buff);
static char *localized_str_tolower(char *buff);
#else
@@ -1510,7 +1510,7 @@ str_numth(char *dest, char *num, int type)
}
/* ----------
- * Convert string to upper-string. Input string is modified in place.
+ * Convert string to upper case. Input string is modified in place.
* ----------
*/
static char *
@@ -1531,7 +1531,7 @@ str_toupper(char *buff)
}
/* ----------
- * Convert string to lower-string. Input string is modified in place.
+ * Convert string to lower case. Input string is modified in place.
* ----------
*/
static char *
@@ -1553,7 +1553,8 @@ str_tolower(char *buff)
#ifdef USE_WIDE_UPPER_LOWER
/* ----------
- * Convert localized string to upper string. Input string is modified in place.
+ * Convert localized string to upper case.
+ * Input string may be modified in place ... or we might make a copy.
* ----------
*/
static char *
@@ -1579,7 +1580,8 @@ localized_str_toupper(char *buff)
}
/* ----------
- * Convert localized string to upper string. Input string is modified in place.
+ * Convert localized string to lower case.
+ * Input string may be modified in place ... or we might make a copy.
* ----------
*/
static char *
@@ -2107,19 +2109,16 @@ dch_time(int arg, char *inout, int suf, bool is_to_char, bool is_interval,
INVALID_FOR_INTERVAL;
if (is_to_char && tmtcTzn(tmtc))
{
- int siz = strlen(tmtcTzn(tmtc));
-
if (arg == DCH_TZ)
strcpy(inout, tmtcTzn(tmtc));
else
{
- char *p = palloc(siz);
+ char *p = pstrdup(tmtcTzn(tmtc));
- strcpy(p, tmtcTzn(tmtc));
strcpy(inout, str_tolower(p));
pfree(p);
}
- return siz;
+ return strlen(inout);
}
else if (!is_to_char)
ereport(ERROR,
@@ -3624,7 +3623,7 @@ static char *
fill_str(char *str, int c, int max)
{
memset(str, c, max);
- *(str + max + 1) = '\0';
+ *(str + max) = '\0';
return str;
}
@@ -4798,10 +4797,9 @@ NUM_processor(FormatNode *node, NUMDesc *Num, char *inout, char *number,
#define NUM_TOCHAR_prepare \
do { \
len = VARSIZE(fmt) - VARHDRSZ; \
- if (len <= 0) \
+ if (len <= 0 || len >= (INT_MAX-VARHDRSZ)/NUM_MAX_ITEM_SIZ) \
return DirectFunctionCall1(textin, CStringGetDatum("")); \
- result = (text *) palloc( (len * NUM_MAX_ITEM_SIZ) + 1 + VARHDRSZ); \
- memset(result, 0, (len * NUM_MAX_ITEM_SIZ) + 1 + VARHDRSZ ); \
+ result = (text *) palloc0((len * NUM_MAX_ITEM_SIZ) + 1 + VARHDRSZ); \
format = NUM_cache(len, &Num, VARDATA(fmt), &shouldFree); \
} while (0)
@@ -4811,30 +4809,19 @@ do { \
*/
#define NUM_TOCHAR_finish \
do { \
- NUM_processor(format, &Num, VARDATA(result), \
- numstr, plen, sign, true); \
- pfree(orgnum); \
+ NUM_processor(format, &Num, VARDATA(result), numstr, plen, sign, true); \
\
- if (shouldFree) \
- pfree(format); \
+ if (shouldFree) \
+ pfree(format); \
\
- /*
- * for result is allocated max memory, which current format-picture\
- * needs, now it must be re-allocate to result real size \
+ /* \
+ * Convert null-terminated representation of result to standard text. \
+ * The result is usually much bigger than it needs to be, but there \
+ * seems little point in realloc'ing it smaller. \
*/ \
- if (!(len = strlen(VARDATA(result)))) \
- { \
- pfree(result); \
- PG_RETURN_NULL(); \
- } \
- \
- result_tmp = result; \
- result = (text *) palloc(len + VARHDRSZ); \
- \
- memcpy(VARDATA(result), VARDATA(result_tmp), len); \
- SET_VARSIZE(result, len + VARHDRSZ); \
- pfree(result_tmp); \
-} while(0)
+ len = strlen(VARDATA(result)); \
+ SET_VARSIZE(result, len + VARHDRSZ); \
+} while (0)
/* -------------------
* NUMERIC to_number() (convert string to numeric)
@@ -4856,7 +4843,7 @@ numeric_to_number(PG_FUNCTION_ARGS)
len = VARSIZE(fmt) - VARHDRSZ;
- if (len <= 0)
+ if (len <= 0 || len >= INT_MAX/NUM_MAX_ITEM_SIZ)
PG_RETURN_NULL();
format = NUM_cache(len, &Num, VARDATA(fmt), &shouldFree);
@@ -4891,8 +4878,7 @@ numeric_to_char(PG_FUNCTION_ARGS)
text *fmt = PG_GETARG_TEXT_P(1);
NUMDesc Num;
FormatNode *format;
- text *result,
- *result_tmp;
+ text *result;
bool shouldFree;
int len = 0,
plen = 0,
@@ -4915,7 +4901,6 @@ numeric_to_char(PG_FUNCTION_ARGS)
numstr = orgnum =
int_to_roman(DatumGetInt32(DirectFunctionCall1(numeric_int4,
NumericGetDatum(x))));
- pfree(x);
}
else
{
@@ -4934,9 +4919,6 @@ numeric_to_char(PG_FUNCTION_ARGS)
val = DatumGetNumeric(DirectFunctionCall2(numeric_mul,
NumericGetDatum(value),
NumericGetDatum(x)));
- pfree(x);
- pfree(a);
- pfree(b);
Num.pre += Num.multi;
}
@@ -4945,10 +4927,9 @@ numeric_to_char(PG_FUNCTION_ARGS)
Int32GetDatum(Num.post)));
orgnum = DatumGetCString(DirectFunctionCall1(numeric_out,
NumericGetDatum(x)));
- pfree(x);
if (*orgnum == '-')
- { /* < 0 */
+ {
sign = '-';
numstr = orgnum + 1;
}
@@ -4966,13 +4947,10 @@ numeric_to_char(PG_FUNCTION_ARGS)
plen = Num.pre - len;
else if (len > Num.pre)
{
- fill_str(numstr, '#', Num.pre);
+ numstr = (char *) palloc(Num.pre + Num.post + 2);
+ fill_str(numstr, '#', Num.pre + Num.post + 1);
*(numstr + Num.pre) = '.';
- fill_str(numstr + 1 + Num.pre, '#', Num.post);
}
-
- if (IS_MULTI(&Num))
- pfree(val);
}
NUM_TOCHAR_finish;
@@ -4990,8 +4968,7 @@ int4_to_char(PG_FUNCTION_ARGS)
text *fmt = PG_GETARG_TEXT_P(1);
NUMDesc Num;
FormatNode *format;
- text *result,
- *result_tmp;
+ text *result;
bool shouldFree;
int len = 0,
plen = 0,
@@ -5019,40 +4996,34 @@ int4_to_char(PG_FUNCTION_ARGS)
orgnum = DatumGetCString(DirectFunctionCall1(int4out,
Int32GetDatum(value)));
}
- len = strlen(orgnum);
if (*orgnum == '-')
- { /* < 0 */
+ {
sign = '-';
- --len;
+ orgnum++;
}
else
sign = '+';
+ len = strlen(orgnum);
if (Num.post)
{
- int i;
-
numstr = (char *) palloc(len + Num.post + 2);
- strcpy(numstr, orgnum + (*orgnum == '-' ? 1 : 0));
+ strcpy(numstr, orgnum);
*(numstr + len) = '.';
-
- for (i = len + 1; i <= len + Num.post; i++)
- *(numstr + i) = '0';
+ memset(numstr + len + 1, '0', Num.post);
*(numstr + len + Num.post + 1) = '\0';
- pfree(orgnum);
- orgnum = numstr;
}
else
- numstr = orgnum + (*orgnum == '-' ? 1 : 0);
+ numstr = orgnum;
if (Num.pre > len)
plen = Num.pre - len;
else if (len > Num.pre)
{
- fill_str(numstr, '#', Num.pre);
+ numstr = (char *) palloc(Num.pre + Num.post + 2);
+ fill_str(numstr, '#', Num.pre + Num.post + 1);
*(numstr + Num.pre) = '.';
- fill_str(numstr + 1 + Num.pre, '#', Num.post);
}
}
@@ -5071,8 +5042,7 @@ int8_to_char(PG_FUNCTION_ARGS)
text *fmt = PG_GETARG_TEXT_P(1);
NUMDesc Num;
FormatNode *format;
- text *result,
- *result_tmp;
+ text *result;
bool shouldFree;
int len = 0,
plen = 0,
@@ -5106,40 +5076,34 @@ int8_to_char(PG_FUNCTION_ARGS)
orgnum = DatumGetCString(DirectFunctionCall1(int8out,
Int64GetDatum(value)));
- len = strlen(orgnum);
if (*orgnum == '-')
- { /* < 0 */
+ {
sign = '-';
- --len;
+ orgnum++;
}
else
sign = '+';
+ len = strlen(orgnum);
if (Num.post)
{
- int i;
-
numstr = (char *) palloc(len + Num.post + 2);
- strcpy(numstr, orgnum + (*orgnum == '-' ? 1 : 0));
+ strcpy(numstr, orgnum);
*(numstr + len) = '.';
-
- for (i = len + 1; i <= len + Num.post; i++)
- *(numstr + i) = '0';
+ memset(numstr + len + 1, '0', Num.post);
*(numstr + len + Num.post + 1) = '\0';
- pfree(orgnum);
- orgnum = numstr;
}
else
- numstr = orgnum + (*orgnum == '-' ? 1 : 0);
+ numstr = orgnum;
if (Num.pre > len)
plen = Num.pre - len;
else if (len > Num.pre)
{
- fill_str(numstr, '#', Num.pre);
+ numstr = (char *) palloc(Num.pre + Num.post + 2);
+ fill_str(numstr, '#', Num.pre + Num.post + 1);
*(numstr + Num.pre) = '.';
- fill_str(numstr + 1 + Num.pre, '#', Num.post);
}
}
@@ -5158,8 +5122,7 @@ float4_to_char(PG_FUNCTION_ARGS)
text *fmt = PG_GETARG_TEXT_P(1);
NUMDesc Num;
FormatNode *format;
- text *result,
- *result_tmp;
+ text *result;
bool shouldFree;
int len = 0,
plen = 0,
@@ -5214,9 +5177,9 @@ float4_to_char(PG_FUNCTION_ARGS)
plen = Num.pre - len;
else if (len > Num.pre)
{
- fill_str(numstr, '#', Num.pre);
+ numstr = (char *) palloc(Num.pre + Num.post + 2);
+ fill_str(numstr, '#', Num.pre + Num.post + 1);
*(numstr + Num.pre) = '.';
- fill_str(numstr + 1 + Num.pre, '#', Num.post);
}
}
@@ -5235,8 +5198,7 @@ float8_to_char(PG_FUNCTION_ARGS)
text *fmt = PG_GETARG_TEXT_P(1);
NUMDesc Num;
FormatNode *format;
- text *result,
- *result_tmp;
+ text *result;
bool shouldFree;
int len = 0,
plen = 0,
@@ -5289,9 +5251,9 @@ float8_to_char(PG_FUNCTION_ARGS)
plen = Num.pre - len;
else if (len > Num.pre)
{
- fill_str(numstr, '#', Num.pre);
+ numstr = (char *) palloc(Num.pre + Num.post + 2);
+ fill_str(numstr, '#', Num.pre + Num.post + 1);
*(numstr + Num.pre) = '.';
- fill_str(numstr + 1 + Num.pre, '#', Num.post);
}
}
--
GitLab