Avoid integer overflow in hstore_to_json().
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Fri, 21 Feb 2014 13:43:31 +0000 (15:43 +0200)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Fri, 21 Feb 2014 13:47:22 +0000 (15:47 +0200)
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.

contrib/hstore/hstore_io.c

index 6dd3f7c24eb647de343376c8bd595bf9e5982ce1..65e4438d3225d49ab9eb61f7b0ec627824d08f67 100644 (file)
@@ -1245,77 +1245,49 @@ Datum
 hstore_to_json_loose(PG_FUNCTION_ARGS)
 {
    HStore     *in = PG_GETARG_HS(0);
-   int         buflen,
-               i;
+   int         i;
    int         count = HS_COUNT(in);
-   char       *out,
-              *ptr;
    char       *base = STRPTR(in);
    HEntry     *entries = ARRPTR(in);
    bool        is_number;
-   StringInfo  src,
+   StringInfoData tmp,
                dst;
 
    if (count == 0)
        PG_RETURN_TEXT_P(cstring_to_text_with_len("{}",2));
 
-   buflen = 3;
+   initStringInfo(&tmp);
+   initStringInfo(&dst);
 
-   /*
-    * Formula adjusted slightly from the logic in hstore_out. We have to take
-    * account of out treatment of booleans to be a bit more pessimistic about
-    * the length of values.
-    */
+   appendStringInfoChar(&dst, '{');
 
    for (i = 0; i < count; i++)
    {
-       /* include "" and colon-space and comma-space */
-       buflen += 6 + 2 * HS_KEYLEN(entries, i);
-       /* include "" only if nonnull */
-       buflen += 3 + (HS_VALISNULL(entries, i)
-                      ? 1
-                      : 2 * HS_VALLEN(entries, i));
-   }
-
-   out = ptr = palloc(buflen);
-
-   src = makeStringInfo();
-   dst = makeStringInfo();
-
-   *ptr++ = '{';
-
-   for (i = 0; i < count; i++)
-   {
-       resetStringInfo(src);
-       resetStringInfo(dst);
-       appendBinaryStringInfo(src, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
-       escape_json(dst, src->data);
-       strncpy(ptr, dst->data, dst->len);
-       ptr += dst->len;
-       *ptr++ = ':';
-       *ptr++ = ' ';
-       resetStringInfo(dst);
+       resetStringInfo(&tmp);
+       appendBinaryStringInfo(&tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
+       escape_json(&dst, tmp.data);
+       appendStringInfoString(&dst, ": ");
        if (HS_VALISNULL(entries, i))
-           appendStringInfoString(dst, "null");
+           appendStringInfoString(&dst, "null");
        /* guess that values of 't' or 'f' are booleans */
        else if (HS_VALLEN(entries, i) == 1 && *(HS_VAL(entries, base, i)) == 't')
-           appendStringInfoString(dst, "true");
+           appendStringInfoString(&dst, "true");
        else if (HS_VALLEN(entries, i) == 1 && *(HS_VAL(entries, base, i)) == 'f')
-           appendStringInfoString(dst, "false");
+           appendStringInfoString(&dst, "false");
        else
        {
            is_number = false;
-           resetStringInfo(src);
-           appendBinaryStringInfo(src, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
+           resetStringInfo(&tmp);
+           appendBinaryStringInfo(&tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
 
            /*
             * don't treat something with a leading zero followed by another
             * digit as numeric - could be a zip code or similar
             */
-           if (src->len > 0 &&
-               !(src->data[0] == '0' &&
-                 isdigit((unsigned char) src->data[1])) &&
-               strspn(src->data, "+-0123456789Ee.") == src->len)
+           if (tmp.len > 0 &&
+               !(tmp.data[0] == '0' &&
+                 isdigit((unsigned char) tmp.data[1])) &&
+               strspn(tmp.data, "+-0123456789Ee.") == tmp.len)
            {
                /*
                 * might be a number. See if we can input it as a numeric
@@ -1324,7 +1296,7 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
                char       *endptr = "junk";
                long        lval;
 
-               lval = strtol(src->data, &endptr, 10);
+               lval = strtol(tmp.data, &endptr, 10);
                (void) lval;
                if (*endptr == '\0')
                {
@@ -1339,30 +1311,24 @@ hstore_to_json_loose(PG_FUNCTION_ARGS)
                    /* not an int - try a double */
                    double      dval;
 
-                   dval = strtod(src->data, &endptr);
+                   dval = strtod(tmp.data, &endptr);
                    (void) dval;
                    if (*endptr == '\0')
                        is_number = true;
                }
            }
            if (is_number)
-               appendBinaryStringInfo(dst, src->data, src->len);
+               appendBinaryStringInfo(&dst, tmp.data, tmp.len);
            else
-               escape_json(dst, src->data);
+               escape_json(&dst, tmp.data);
        }
-       strncpy(ptr, dst->data, dst->len);
-       ptr += dst->len;
 
        if (i + 1 != count)
-       {
-           *ptr++ = ',';
-           *ptr++ = ' ';
-       }
+           appendStringInfoString(&dst, ", ");
    }
-   *ptr++ = '}';
-   *ptr = '\0';
+   appendStringInfoChar(&dst, '}');
 
-   PG_RETURN_TEXT_P(cstring_to_text(out));
+   PG_RETURN_TEXT_P(cstring_to_text(dst.data));
 }
 
 PG_FUNCTION_INFO_V1(hstore_to_json);
@@ -1371,74 +1337,40 @@ Datum
 hstore_to_json(PG_FUNCTION_ARGS)
 {
    HStore     *in = PG_GETARG_HS(0);
-   int         buflen,
-               i;
+   int         i;
    int         count = HS_COUNT(in);
-   char       *out,
-              *ptr;
    char       *base = STRPTR(in);
    HEntry     *entries = ARRPTR(in);
-   StringInfo  src,
+   StringInfoData tmp,
                dst;
 
    if (count == 0)
        PG_RETURN_TEXT_P(cstring_to_text_with_len("{}",2));
 
-   buflen = 3;
+   initStringInfo(&tmp);
+   initStringInfo(&dst);
 
-   /*
-    * Formula adjusted slightly from the logic in hstore_out. We have to take
-    * account of out treatment of booleans to be a bit more pessimistic about
-    * the length of values.
-    */
+   appendStringInfoChar(&dst, '{');
 
    for (i = 0; i < count; i++)
    {
-       /* include "" and colon-space and comma-space */
-       buflen += 6 + 2 * HS_KEYLEN(entries, i);
-       /* include "" only if nonnull */
-       buflen += 3 + (HS_VALISNULL(entries, i)
-                      ? 1
-                      : 2 * HS_VALLEN(entries, i));
-   }
-
-   out = ptr = palloc(buflen);
-
-   src = makeStringInfo();
-   dst = makeStringInfo();
-
-   *ptr++ = '{';
-
-   for (i = 0; i < count; i++)
-   {
-       resetStringInfo(src);
-       resetStringInfo(dst);
-       appendBinaryStringInfo(src, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
-       escape_json(dst, src->data);
-       strncpy(ptr, dst->data, dst->len);
-       ptr += dst->len;
-       *ptr++ = ':';
-       *ptr++ = ' ';
-       resetStringInfo(dst);
+       resetStringInfo(&tmp);
+       appendBinaryStringInfo(&tmp, HS_KEY(entries, base, i), HS_KEYLEN(entries, i));
+       escape_json(&dst, tmp.data);
+       appendStringInfoString(&dst, ": ");
        if (HS_VALISNULL(entries, i))
-           appendStringInfoString(dst, "null");
+           appendStringInfoString(&dst, "null");
        else
        {
-           resetStringInfo(src);
-           appendBinaryStringInfo(src, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
-           escape_json(dst, src->data);
+           resetStringInfo(&tmp);
+           appendBinaryStringInfo(&tmp, HS_VAL(entries, base, i), HS_VALLEN(entries, i));
+           escape_json(&dst, tmp.data);
        }
-       strncpy(ptr, dst->data, dst->len);
-       ptr += dst->len;
 
        if (i + 1 != count)
-       {
-           *ptr++ = ',';
-           *ptr++ = ' ';
-       }
+           appendStringInfoString(&dst, ", ");
    }
-   *ptr++ = '}';
-   *ptr = '\0';
+   appendStringInfoChar(&dst, '}');
 
-   PG_RETURN_TEXT_P(cstring_to_text(out));
+   PG_RETURN_TEXT_P(cstring_to_text(dst.data));
 }