Skip to content

Commit f44290b

Browse files
tglsfdctglsfdc
committed
Avoid integer overflow and buffer overrun in hstore_to_json().
This back-patches commit 0c5783f into the 9.3 branch. At the time, Heikki just thought he was fixing an unlikely integer-overflow scenario, but in point of fact the original coding was hopelessly broken: it supposed that escape_json never enlarges the data more than 2X, which is wrong on its face. The revised code eliminates making any a-priori assumptions about the output length. Per report from Saul Costa. The bogus code doesn't exist before 9.3, so no other branches need fixing.
1 parent f883001 commit f44290b

File tree

1 file changed

+41
-109
lines changed

1 file changed

+41
-109
lines changed

‎contrib/hstore/hstore_io.c

Lines changed: 41 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -1249,77 +1249,49 @@ Datum
12491249
hstore_to_json_loose(PG_FUNCTION_ARGS)
12501250
{
12511251
HStore *in = PG_GETARG_HS(0);
1252-
int buflen,
1253-
i;
1252+
int i;
12541253
int count = HS_COUNT(in);
1255-
char *out,
1256-
*ptr;
12571254
char *base = STRPTR(in);
12581255
HEntry *entries = ARRPTR(in);
12591256
bool is_number;
1260-
StringInfo src,
1257+
StringInfoData tmp,
12611258
dst;
12621259

12631260
if (count == 0)
12641261
PG_RETURN_TEXT_P(cstring_to_text_with_len("{}",2));
12651262

1266-
buflen = 3;
1263+
initStringInfo(&tmp);
1264+
initStringInfo(&dst);
12671265

1268-
/*
1269-
* Formula adjusted slightly from the logic in hstore_out. We have to take
1270-
* account of out treatment of booleans to be a bit more pessimistic about
1271-
* the length of values.
1272-
*/
1266+
appendStringInfoChar(&dst, '{');
12731267

12741268
for (i = 0; i < count; i++)
12751269
{
1276-
/* include "" and colon-space and comma-space */
1277-
buflen += 6 + 2 * HS_KEYLEN(entries, i);
1278-
/* include "" only if nonnull */
1279-
buflen += 3 + (HS_VALISNULL(entries, i)
1280-
? 1
1281-
: 2 * HS_VALLEN(entries, i));
1282-
}
1283-
1284-
out = ptr = palloc(buflen);
1285-
1286-
src = makeStringInfo();
1287-
dst = makeStringInfo();
1288-
1289-
*ptr++ = '{';
1290-
1291-
for (i = 0; i < count; i++)
1292-
{
1293-
resetStringInfo(src);
1294-
resetStringInfo(dst);
1295-
appendBinaryStringInfo(src, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1296-
escape_json(dst, src->data);
1297-
strncpy(ptr, dst->data, dst->len);
1298-
ptr += dst->len;
1299-
*ptr++ = ':';
1300-
*ptr++ = ' ';
1301-
resetStringInfo(dst);
1270+
resetStringInfo(&tmp);
1271+
appendBinaryStringInfo(&tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1272+
escape_json(&dst, tmp.data);
1273+
appendStringInfoString(&dst, ": ");
13021274
if (HS_VALISNULL(entries, i))
1303-
appendStringInfoString(dst, "null");
1275+
appendStringInfoString(&dst, "null");
13041276
/* guess that values of 't' or 'f' are booleans */
13051277
else if (HS_VALLEN(entries, i) == 1 && *(HS_VAL(entries, base, i)) == 't')
1306-
appendStringInfoString(dst, "true");
1278+
appendStringInfoString(&dst, "true");
13071279
else if (HS_VALLEN(entries, i) == 1 && *(HS_VAL(entries, base, i)) == 'f')
1308-
appendStringInfoString(dst, "false");
1280+
appendStringInfoString(&dst, "false");
13091281
else
13101282
{
13111283
is_number = false;
1312-
resetStringInfo(src);
1313-
appendBinaryStringInfo(src, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
1284+
resetStringInfo(&tmp);
1285+
appendBinaryStringInfo(&tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
13141286

13151287
/*
13161288
* don't treat something with a leading zero followed by another
13171289
* digit as numeric - could be a zip code or similar
13181290
*/
1319-
if (src->len > 0 &&
1320-
!(src->data[0] == '0' &&
1321-
isdigit((unsigned char) src->data[1])) &&
1322-
strspn(src->data, "+-0123456789Ee.") == src->len)
1291+
if (tmp.len > 0 &&
1292+
!(tmp.data[0] == '0' &&
1293+
isdigit((unsigned char) tmp.data[1])) &&
1294+
strspn(tmp.data, "+-0123456789Ee.") == tmp.len)
13231295
{
13241296
/*
13251297
* might be a number. See if we can input it as a numeric
@@ -1328,7 +1300,7 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
13281300
char *endptr = "junk";
13291301
long lval;
13301302

1331-
lval = strtol(src->data, &endptr, 10);
1303+
lval = strtol(tmp.data, &endptr, 10);
13321304
(void) lval;
13331305
if (*endptr == '\0')
13341306
{
@@ -1343,30 +1315,24 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
13431315
/* not an int - try a double */
13441316
double dval;
13451317

1346-
dval = strtod(src->data, &endptr);
1318+
dval = strtod(tmp.data, &endptr);
13471319
(void) dval;
13481320
if (*endptr == '\0')
13491321
is_number = true;
13501322
}
13511323
}
13521324
if (is_number)
1353-
appendBinaryStringInfo(dst, src->data, src->len);
1325+
appendBinaryStringInfo(&dst, tmp.data, tmp.len);
13541326
else
1355-
escape_json(dst, src->data);
1327+
escape_json(&dst, tmp.data);
13561328
}
1357-
strncpy(ptr, dst->data, dst->len);
1358-
ptr += dst->len;
13591329

13601330
if (i + 1 != count)
1361-
{
1362-
*ptr++ = ',';
1363-
*ptr++ = ' ';
1364-
}
1331+
appendStringInfoString(&dst, ", ");
13651332
}
1366-
*ptr++ = '}';
1367-
*ptr = '\0';
1333+
appendStringInfoChar(&dst, '}');
13681334

1369-
PG_RETURN_TEXT_P(cstring_to_text(out));
1335+
PG_RETURN_TEXT_P(cstring_to_text(dst.data));
13701336
}
13711337

13721338
PG_FUNCTION_INFO_V1(hstore_to_json);
@@ -1375,74 +1341,40 @@ Datum
13751341
hstore_to_json(PG_FUNCTION_ARGS)
13761342
{
13771343
HStore *in = PG_GETARG_HS(0);
1378-
int buflen,
1379-
i;
1344+
int i;
13801345
int count = HS_COUNT(in);
1381-
char *out,
1382-
*ptr;
13831346
char *base = STRPTR(in);
13841347
HEntry *entries = ARRPTR(in);
1385-
StringInfo src,
1348+
StringInfoData tmp,
13861349
dst;
13871350

13881351
if (count == 0)
13891352
PG_RETURN_TEXT_P(cstring_to_text_with_len("{}",2));
13901353

1391-
buflen = 3;
1354+
initStringInfo(&tmp);
1355+
initStringInfo(&dst);
13921356

1393-
/*
1394-
* Formula adjusted slightly from the logic in hstore_out. We have to take
1395-
* account of out treatment of booleans to be a bit more pessimistic about
1396-
* the length of values.
1397-
*/
1357+
appendStringInfoChar(&dst, '{');
13981358

13991359
for (i = 0; i < count; i++)
14001360
{
1401-
/* include "" and colon-space and comma-space */
1402-
buflen += 6 + 2 * HS_KEYLEN(entries, i);
1403-
/* include "" only if nonnull */
1404-
buflen += 3 + (HS_VALISNULL(entries, i)
1405-
? 1
1406-
: 2 * HS_VALLEN(entries, i));
1407-
}
1408-
1409-
out = ptr = palloc(buflen);
1410-
1411-
src = makeStringInfo();
1412-
dst = makeStringInfo();
1413-
1414-
*ptr++ = '{';
1415-
1416-
for (i = 0; i < count; i++)
1417-
{
1418-
resetStringInfo(src);
1419-
resetStringInfo(dst);
1420-
appendBinaryStringInfo(src, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1421-
escape_json(dst, src->data);
1422-
strncpy(ptr, dst->data, dst->len);
1423-
ptr += dst->len;
1424-
*ptr++ = ':';
1425-
*ptr++ = ' ';
1426-
resetStringInfo(dst);
1361+
resetStringInfo(&tmp);
1362+
appendBinaryStringInfo(&tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1363+
escape_json(&dst, tmp.data);
1364+
appendStringInfoString(&dst, ": ");
14271365
if (HS_VALISNULL(entries, i))
1428-
appendStringInfoString(dst, "null");
1366+
appendStringInfoString(&dst, "null");
14291367
else
14301368
{
1431-
resetStringInfo(src);
1432-
appendBinaryStringInfo(src, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
1433-
escape_json(dst, src->data);
1369+
resetStringInfo(&tmp);
1370+
appendBinaryStringInfo(&tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
1371+
escape_json(&dst, tmp.data);
14341372
}
1435-
strncpy(ptr, dst->data, dst->len);
1436-
ptr += dst->len;
14371373

14381374
if (i + 1 != count)
1439-
{
1440-
*ptr++ = ',';
1441-
*ptr++ = ' ';
1442-
}
1375+
appendStringInfoString(&dst, ", ");
14431376
}
1444-
*ptr++ = '}';
1445-
*ptr = '\0';
1377+
appendStringInfoChar(&dst, '}');
14461378

1447-
PG_RETURN_TEXT_P(cstring_to_text(out));
1379+
PG_RETURN_TEXT_P(cstring_to_text(dst.data));
14481380
}

0 commit comments

Comments
 (0)