Skip to content

Commit 0c5783f

Browse files
hlinnakahlinnaka
committed
Avoid integer overflow in hstore_to_json().
The length of the output buffer was calculated based on the size of the argument hstore. On a sizeof(int) == 4 platform and a huge argument, it could overflow, causing a too small buffer to be allocated. Refactor the function to use a StringInfo instead of pre-allocating the buffer. Makes it shorter and more readable, too.
1 parent 8c059df commit 0c5783f

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
@@ -1245,77 +1245,49 @@ Datum
12451245
hstore_to_json_loose(PG_FUNCTION_ARGS)
12461246
{
12471247
HStore *in = PG_GETARG_HS(0);
1248-
int buflen,
1249-
i;
1248+
int i;
12501249
int count = HS_COUNT(in);
1251-
char *out,
1252-
*ptr;
12531250
char *base = STRPTR(in);
12541251
HEntry *entries = ARRPTR(in);
12551252
bool is_number;
1256-
StringInfo src,
1253+
StringInfoData tmp,
12571254
dst;
12581255

12591256
if (count == 0)
12601257
PG_RETURN_TEXT_P(cstring_to_text_with_len("{}",2));
12611258

1262-
buflen = 3;
1259+
initStringInfo(&tmp);
1260+
initStringInfo(&dst);
12631261

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

12701264
for (i = 0; i < count; i++)
12711265
{
1272-
/* include "" and colon-space and comma-space */
1273-
buflen += 6 + 2 * HS_KEYLEN(entries, i);
1274-
/* include "" only if nonnull */
1275-
buflen += 3 + (HS_VALISNULL(entries, i)
1276-
? 1
1277-
: 2 * HS_VALLEN(entries, i));
1278-
}
1279-
1280-
out = ptr = palloc(buflen);
1281-
1282-
src = makeStringInfo();
1283-
dst = makeStringInfo();
1284-
1285-
*ptr++ = '{';
1286-
1287-
for (i = 0; i < count; i++)
1288-
{
1289-
resetStringInfo(src);
1290-
resetStringInfo(dst);
1291-
appendBinaryStringInfo(src, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1292-
escape_json(dst, src->data);
1293-
strncpy(ptr, dst->data, dst->len);
1294-
ptr += dst->len;
1295-
*ptr++ = ':';
1296-
*ptr++ = ' ';
1297-
resetStringInfo(dst);
1266+
resetStringInfo(&tmp);
1267+
appendBinaryStringInfo(&tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1268+
escape_json(&dst, tmp.data);
1269+
appendStringInfoString(&dst, ": ");
12981270
if (HS_VALISNULL(entries, i))
1299-
appendStringInfoString(dst, "null");
1271+
appendStringInfoString(&dst, "null");
13001272
/* guess that values of 't' or 'f' are booleans */
13011273
else if (HS_VALLEN(entries, i) == 1 && *(HS_VAL(entries, base, i)) == 't')
1302-
appendStringInfoString(dst, "true");
1274+
appendStringInfoString(&dst, "true");
13031275
else if (HS_VALLEN(entries, i) == 1 && *(HS_VAL(entries, base, i)) == 'f')
1304-
appendStringInfoString(dst, "false");
1276+
appendStringInfoString(&dst, "false");
13051277
else
13061278
{
13071279
is_number = false;
1308-
resetStringInfo(src);
1309-
appendBinaryStringInfo(src, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
1280+
resetStringInfo(&tmp);
1281+
appendBinaryStringInfo(&tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
13101282

13111283
/*
13121284
* don't treat something with a leading zero followed by another
13131285
* digit as numeric - could be a zip code or similar
13141286
*/
1315-
if (src->len > 0 &&
1316-
!(src->data[0] == '0' &&
1317-
isdigit((unsigned char) src->data[1])) &&
1318-
strspn(src->data, "+-0123456789Ee.") == src->len)
1287+
if (tmp.len > 0 &&
1288+
!(tmp.data[0] == '0' &&
1289+
isdigit((unsigned char) tmp.data[1])) &&
1290+
strspn(tmp.data, "+-0123456789Ee.") == tmp.len)
13191291
{
13201292
/*
13211293
* might be a number. See if we can input it as a numeric
@@ -1324,7 +1296,7 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
13241296
char *endptr = "junk";
13251297
long lval;
13261298

1327-
lval = strtol(src->data, &endptr, 10);
1299+
lval = strtol(tmp.data, &endptr, 10);
13281300
(void) lval;
13291301
if (*endptr == '\0')
13301302
{
@@ -1339,30 +1311,24 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
13391311
/* not an int - try a double */
13401312
double dval;
13411313

1342-
dval = strtod(src->data, &endptr);
1314+
dval = strtod(tmp.data, &endptr);
13431315
(void) dval;
13441316
if (*endptr == '\0')
13451317
is_number = true;
13461318
}
13471319
}
13481320
if (is_number)
1349-
appendBinaryStringInfo(dst, src->data, src->len);
1321+
appendBinaryStringInfo(&dst, tmp.data, tmp.len);
13501322
else
1351-
escape_json(dst, src->data);
1323+
escape_json(&dst, tmp.data);
13521324
}
1353-
strncpy(ptr, dst->data, dst->len);
1354-
ptr += dst->len;
13551325

13561326
if (i + 1 != count)
1357-
{
1358-
*ptr++ = ',';
1359-
*ptr++ = ' ';
1360-
}
1327+
appendStringInfoString(&dst, ", ");
13611328
}
1362-
*ptr++ = '}';
1363-
*ptr = '\0';
1329+
appendStringInfoChar(&dst, '}');
13641330

1365-
PG_RETURN_TEXT_P(cstring_to_text(out));
1331+
PG_RETURN_TEXT_P(cstring_to_text(dst.data));
13661332
}
13671333

13681334
PG_FUNCTION_INFO_V1(hstore_to_json);
@@ -1371,74 +1337,40 @@ Datum
13711337
hstore_to_json(PG_FUNCTION_ARGS)
13721338
{
13731339
HStore *in = PG_GETARG_HS(0);
1374-
int buflen,
1375-
i;
1340+
int i;
13761341
int count = HS_COUNT(in);
1377-
char *out,
1378-
*ptr;
13791342
char *base = STRPTR(in);
13801343
HEntry *entries = ARRPTR(in);
1381-
StringInfo src,
1344+
StringInfoData tmp,
13821345
dst;
13831346

13841347
if (count == 0)
13851348
PG_RETURN_TEXT_P(cstring_to_text_with_len("{}",2));
13861349

1387-
buflen = 3;
1350+
initStringInfo(&tmp);
1351+
initStringInfo(&dst);
13881352

1389-
/*
1390-
* Formula adjusted slightly from the logic in hstore_out. We have to take
1391-
* account of out treatment of booleans to be a bit more pessimistic about
1392-
* the length of values.
1393-
*/
1353+
appendStringInfoChar(&dst, '{');
13941354

13951355
for (i = 0; i < count; i++)
13961356
{
1397-
/* include "" and colon-space and comma-space */
1398-
buflen += 6 + 2 * HS_KEYLEN(entries, i);
1399-
/* include "" only if nonnull */
1400-
buflen += 3 + (HS_VALISNULL(entries, i)
1401-
? 1
1402-
: 2 * HS_VALLEN(entries, i));
1403-
}
1404-
1405-
out = ptr = palloc(buflen);
1406-
1407-
src = makeStringInfo();
1408-
dst = makeStringInfo();
1409-
1410-
*ptr++ = '{';
1411-
1412-
for (i = 0; i < count; i++)
1413-
{
1414-
resetStringInfo(src);
1415-
resetStringInfo(dst);
1416-
appendBinaryStringInfo(src, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1417-
escape_json(dst, src->data);
1418-
strncpy(ptr, dst->data, dst->len);
1419-
ptr += dst->len;
1420-
*ptr++ = ':';
1421-
*ptr++ = ' ';
1422-
resetStringInfo(dst);
1357+
resetStringInfo(&tmp);
1358+
appendBinaryStringInfo(&tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
1359+
escape_json(&dst, tmp.data);
1360+
appendStringInfoString(&dst, ": ");
14231361
if (HS_VALISNULL(entries, i))
1424-
appendStringInfoString(dst, "null");
1362+
appendStringInfoString(&dst, "null");
14251363
else
14261364
{
1427-
resetStringInfo(src);
1428-
appendBinaryStringInfo(src, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
1429-
escape_json(dst, src->data);
1365+
resetStringInfo(&tmp);
1366+
appendBinaryStringInfo(&tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
1367+
escape_json(&dst, tmp.data);
14301368
}
1431-
strncpy(ptr, dst->data, dst->len);
1432-
ptr += dst->len;
14331369

14341370
if (i + 1 != count)
1435-
{
1436-
*ptr++ = ',';
1437-
*ptr++ = ' ';
1438-
}
1371+
appendStringInfoString(&dst, ", ");
14391372
}
1440-
*ptr++ = '}';
1441-
*ptr = '\0';
1373+
appendStringInfoChar(&dst, '}');
14421374

1443-
PG_RETURN_TEXT_P(cstring_to_text(out));
1375+
PG_RETURN_TEXT_P(cstring_to_text(dst.data));
14441376
}

0 commit comments

Comments
 (0)