Skip to content

Commit 175c3b8

Browse files
tglsfdctglsfdc
committed
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.
1 parent 01e570e commit 175c3b8

File tree

1 file changed

+49
-86
lines changed

1 file changed

+49
-86
lines changed

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

Lines changed: 49 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* -----------------------------------------------------------------------
22
* formatting.c
33
*
4-
* $Header: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v 1.56.2.3 2005/03/26 00:42:44 tgl Exp $
4+
* $Header: /cvsroot/pgsql/src/backend/utils/adt/formatting.c,v 1.56.2.4 2007/06/29 01:52:21 tgl Exp $
55
*
66
*
77
* Portions Copyright (c) 1999-2002, PostgreSQL Global Development Group
@@ -75,6 +75,7 @@
7575
#include <locale.h>
7676
#include <math.h>
7777
#include <float.h>
78+
#include <limits.h>
7879

7980
#include "utils/builtins.h"
8081
#include "utils/date.h"
@@ -109,8 +110,8 @@
109110
* More is in float.c
110111
* ----------
111112
*/
112-
#define MAXFLOATWIDTH 64
113-
#define MAXDOUBLEWIDTH 128
113+
#define MAXFLOATWIDTH 60
114+
#define MAXDOUBLEWIDTH 500
114115

115116
/* ----------
116117
* External (defined in PgSQL dt.c (timestamp utils))
@@ -1417,7 +1418,7 @@ str_numth(char *dest, char *num, int type)
14171418
}
14181419

14191420
/* ----------
1420-
* Convert string to upper-string. Input string is modified in place.
1421+
* Convert string to upper case. Input string is modified in place.
14211422
* ----------
14221423
*/
14231424
static char *
@@ -1437,7 +1438,7 @@ str_toupper(char *buff)
14371438
}
14381439

14391440
/* ----------
1440-
* Convert string to lower-string. Input string is modified in place.
1441+
* Convert string to lower case. Input string is modified in place.
14411442
* ----------
14421443
*/
14431444
static char *
@@ -1963,19 +1964,16 @@ dch_time(int arg, char *inout, int suf, int flag, FormatNode *node, void *data)
19631964
case DCH_TZ:
19641965
if (flag == TO_CHAR && tmtcTzn(tmtc))
19651966
{
1966-
int siz = strlen(tmtcTzn(tmtc));
1967-
19681967
if (arg == DCH_TZ)
19691968
strcpy(inout, tmtcTzn(tmtc));
19701969
else
19711970
{
1972-
char *p = palloc(siz);
1971+
char *p = pstrdup(tmtcTzn(tmtc));
19731972

1974-
strcpy(p, tmtcTzn(tmtc));
19751973
strcpy(inout, str_tolower(p));
19761974
pfree(p);
19771975
}
1978-
return siz - 1;
1976+
return strlen(inout) - 1;
19791977
}
19801978
else if (flag == FROM_CHAR)
19811979
elog(ERROR, "to_timestamp(): TZ/tz not supported.");
@@ -3155,7 +3153,7 @@ static char *
31553153
fill_str(char *str, int c, int max)
31563154
{
31573155
memset(str, c, max);
3158-
*(str + max + 1) = '\0';
3156+
*(str + max) = '\0';
31593157
return str;
31603158
}
31613159

@@ -4286,9 +4284,9 @@ NUM_processor(FormatNode *node, NUMDesc *Num, char *inout, char *number,
42864284
#define NUM_TOCHAR_prepare \
42874285
do { \
42884286
len = VARSIZE(fmt) - VARHDRSZ; \
4289-
if (len <= 0) \
4287+
if (len <= 0 || len >= (INT_MAX-VARHDRSZ)/NUM_MAX_ITEM_SIZ) \
42904288
return DirectFunctionCall1(textin, CStringGetDatum("")); \
4291-
result = (text *) palloc( (len * NUM_MAX_ITEM_SIZ) + 1 + VARHDRSZ); \
4289+
result = (text *) palloc((len * NUM_MAX_ITEM_SIZ) + 1 + VARHDRSZ); \
42924290
format = NUM_cache(len, &Num, VARDATA(fmt), &shouldFree); \
42934291
} while (0)
42944292

@@ -4300,28 +4298,18 @@ do { \
43004298
do { \
43014299
NUM_processor(format, &Num, VARDATA(result), \
43024300
numstr, plen, sign, TO_CHAR); \
4303-
pfree(orgnum); \
43044301
\
4305-
if (shouldFree) \
4306-
pfree(format); \
4302+
if (shouldFree) \
4303+
pfree(format); \
43074304
\
4308-
/*
4309-
* for result is allocated max memory, which current format-picture\
4310-
* needs, now it must be re-allocate to result real size \
4305+
/* \
4306+
* Convert null-terminated representation of result to standard text. \
4307+
* The result is usually much bigger than it needs to be, but there \
4308+
* seems little point in realloc'ing it smaller. \
43114309
*/ \
4312-
if (!(len = strlen(VARDATA(result)))) \
4313-
{ \
4314-
pfree(result); \
4315-
PG_RETURN_NULL(); \
4316-
} \
4317-
\
4318-
result_tmp = result; \
4319-
result = (text *) palloc( len + 1 + VARHDRSZ); \
4320-
\
4321-
strcpy( VARDATA(result), VARDATA(result_tmp)); \
4322-
VARATT_SIZEP(result) = len + VARHDRSZ; \
4323-
pfree(result_tmp); \
4324-
} while(0)
4310+
len = strlen(VARDATA(result)); \
4311+
VARATT_SIZEP(result) = len + VARHDRSZ; \
4312+
} while (0)
43254313

43264314
/* -------------------
43274315
* NUMERIC to_number() (convert string to numeric)
@@ -4343,7 +4331,7 @@ numeric_to_number(PG_FUNCTION_ARGS)
43434331

43444332
len = VARSIZE(fmt) - VARHDRSZ;
43454333

4346-
if (len <= 0)
4334+
if (len <= 0 || len >= INT_MAX/NUM_MAX_ITEM_SIZ)
43474335
PG_RETURN_NULL();
43484336

43494337
format = NUM_cache(len, &Num, VARDATA(fmt), &shouldFree);
@@ -4378,8 +4366,7 @@ numeric_to_char(PG_FUNCTION_ARGS)
43784366
text *fmt = PG_GETARG_TEXT_P(1);
43794367
NUMDesc Num;
43804368
FormatNode *format;
4381-
text *result,
4382-
*result_tmp;
4369+
text *result;
43834370
bool shouldFree;
43844371
int len = 0,
43854372
plen = 0,
@@ -4402,7 +4389,6 @@ numeric_to_char(PG_FUNCTION_ARGS)
44024389
numstr = orgnum =
44034390
int_to_roman(DatumGetInt32(DirectFunctionCall1(numeric_int4,
44044391
NumericGetDatum(x))));
4405-
pfree(x);
44064392
}
44074393
else
44084394
{
@@ -4421,9 +4407,6 @@ numeric_to_char(PG_FUNCTION_ARGS)
44214407
val = DatumGetNumeric(DirectFunctionCall2(numeric_mul,
44224408
NumericGetDatum(value),
44234409
NumericGetDatum(x)));
4424-
pfree(x);
4425-
pfree(a);
4426-
pfree(b);
44274410
Num.pre += Num.multi;
44284411
}
44294412

@@ -4432,10 +4415,9 @@ numeric_to_char(PG_FUNCTION_ARGS)
44324415
Int32GetDatum(Num.post)));
44334416
orgnum = DatumGetCString(DirectFunctionCall1(numeric_out,
44344417
NumericGetDatum(x)));
4435-
pfree(x);
44364418

44374419
if (*orgnum == '-')
4438-
{ /* < 0 */
4420+
{
44394421
sign = '-';
44404422
numstr = orgnum + 1;
44414423
}
@@ -4454,13 +4436,10 @@ numeric_to_char(PG_FUNCTION_ARGS)
44544436

44554437
else if (len > Num.pre)
44564438
{
4457-
fill_str(numstr, '#', Num.pre);
4439+
numstr = (char *) palloc(Num.pre + Num.post + 2);
4440+
fill_str(numstr, '#', Num.pre + Num.post + 1);
44584441
*(numstr + Num.pre) = '.';
4459-
fill_str(numstr + 1 + Num.pre, '#', Num.post);
44604442
}
4461-
4462-
if (IS_MULTI(&Num))
4463-
pfree(val);
44644443
}
44654444

44664445
NUM_TOCHAR_finish;
@@ -4478,8 +4457,7 @@ int4_to_char(PG_FUNCTION_ARGS)
44784457
text *fmt = PG_GETARG_TEXT_P(1);
44794458
NUMDesc Num;
44804459
FormatNode *format;
4481-
text *result,
4482-
*result_tmp;
4460+
text *result;
44834461
bool shouldFree;
44844462
int len = 0,
44854463
plen = 0,
@@ -4507,40 +4485,34 @@ int4_to_char(PG_FUNCTION_ARGS)
45074485
orgnum = DatumGetCString(DirectFunctionCall1(int4out,
45084486
Int32GetDatum(value)));
45094487
}
4510-
len = strlen(orgnum);
45114488

45124489
if (*orgnum == '-')
4513-
{ /* < 0 */
4490+
{
45144491
sign = '-';
4515-
--len;
4492+
orgnum++;
45164493
}
45174494
else
45184495
sign = '+';
4496+
len = strlen(orgnum);
45194497

45204498
if (Num.post)
45214499
{
4522-
int i;
4523-
45244500
numstr = (char *) palloc(len + Num.post + 2);
4525-
strcpy(numstr, orgnum + (*orgnum == '-' ? 1 : 0));
4501+
strcpy(numstr, orgnum);
45264502
*(numstr + len) = '.';
4527-
4528-
for (i = len + 1; i <= len + Num.post; i++)
4529-
*(numstr + i) = '0';
4503+
memset(numstr + len + 1, '0', Num.post);
45304504
*(numstr + len + Num.post + 1) = '\0';
4531-
pfree(orgnum);
4532-
orgnum = numstr;
45334505
}
45344506
else
4535-
numstr = orgnum + (*orgnum == '-' ? 1 : 0);
4507+
numstr = orgnum;
45364508

45374509
if (Num.pre > len)
45384510
plen = Num.pre - len;
45394511
else if (len > Num.pre)
45404512
{
4541-
fill_str(numstr, '#', Num.pre);
4513+
numstr = (char *) palloc(Num.pre + Num.post + 2);
4514+
fill_str(numstr, '#', Num.pre + Num.post + 1);
45424515
*(numstr + Num.pre) = '.';
4543-
fill_str(numstr + 1 + Num.pre, '#', Num.post);
45444516
}
45454517
}
45464518

@@ -4559,8 +4531,7 @@ int8_to_char(PG_FUNCTION_ARGS)
45594531
text *fmt = PG_GETARG_TEXT_P(1);
45604532
NUMDesc Num;
45614533
FormatNode *format;
4562-
text *result,
4563-
*result_tmp;
4534+
text *result;
45644535
bool shouldFree;
45654536
int len = 0,
45664537
plen = 0,
@@ -4594,40 +4565,34 @@ int8_to_char(PG_FUNCTION_ARGS)
45944565

45954566
orgnum = DatumGetCString(DirectFunctionCall1(int8out,
45964567
Int64GetDatum(value)));
4597-
len = strlen(orgnum);
45984568

45994569
if (*orgnum == '-')
4600-
{ /* < 0 */
4570+
{
46014571
sign = '-';
4602-
--len;
4572+
orgnum++;
46034573
}
46044574
else
46054575
sign = '+';
4576+
len = strlen(orgnum);
46064577

46074578
if (Num.post)
46084579
{
4609-
int i;
4610-
46114580
numstr = (char *) palloc(len + Num.post + 2);
4612-
strcpy(numstr, orgnum + (*orgnum == '-' ? 1 : 0));
4581+
strcpy(numstr, orgnum);
46134582
*(numstr + len) = '.';
4614-
4615-
for (i = len + 1; i <= len + Num.post; i++)
4616-
*(numstr + i) = '0';
4583+
memset(numstr + len + 1, '0', Num.post);
46174584
*(numstr + len + Num.post + 1) = '\0';
4618-
pfree(orgnum);
4619-
orgnum = numstr;
46204585
}
46214586
else
4622-
numstr = orgnum + (*orgnum == '-' ? 1 : 0);
4587+
numstr = orgnum;
46234588

46244589
if (Num.pre > len)
46254590
plen = Num.pre - len;
46264591
else if (len > Num.pre)
46274592
{
4628-
fill_str(numstr, '#', Num.pre);
4593+
numstr = (char *) palloc(Num.pre + Num.post + 2);
4594+
fill_str(numstr, '#', Num.pre + Num.post + 1);
46294595
*(numstr + Num.pre) = '.';
4630-
fill_str(numstr + 1 + Num.pre, '#', Num.post);
46314596
}
46324597
}
46334598

@@ -4646,8 +4611,7 @@ float4_to_char(PG_FUNCTION_ARGS)
46464611
text *fmt = PG_GETARG_TEXT_P(1);
46474612
NUMDesc Num;
46484613
FormatNode *format;
4649-
text *result,
4650-
*result_tmp;
4614+
text *result;
46514615
bool shouldFree;
46524616
int len = 0,
46534617
plen = 0,
@@ -4706,9 +4670,9 @@ float4_to_char(PG_FUNCTION_ARGS)
47064670

47074671
else if (len > Num.pre)
47084672
{
4709-
fill_str(numstr, '#', Num.pre);
4673+
numstr = (char *) palloc(Num.pre + Num.post + 2);
4674+
fill_str(numstr, '#', Num.pre + Num.post + 1);
47104675
*(numstr + Num.pre) = '.';
4711-
fill_str(numstr + 1 + Num.pre, '#', Num.post);
47124676
}
47134677
}
47144678

@@ -4727,8 +4691,7 @@ float8_to_char(PG_FUNCTION_ARGS)
47274691
text *fmt = PG_GETARG_TEXT_P(1);
47284692
NUMDesc Num;
47294693
FormatNode *format;
4730-
text *result,
4731-
*result_tmp;
4694+
text *result;
47324695
bool shouldFree;
47334696
int len = 0,
47344697
plen = 0,
@@ -4785,9 +4748,9 @@ float8_to_char(PG_FUNCTION_ARGS)
47854748

47864749
else if (len > Num.pre)
47874750
{
4788-
fill_str(numstr, '#', Num.pre);
4751+
numstr = (char *) palloc(Num.pre + Num.post + 2);
4752+
fill_str(numstr, '#', Num.pre + Num.post + 1);
47894753
*(numstr + Num.pre) = '.';
4790-
fill_str(numstr + 1 + Num.pre, '#', Num.post);
47914754
}
47924755
}
47934756

0 commit comments

Comments
 (0)