Fix buffer overflows (CVE-2021-45958)

.github/workflows/fuzz.yml

@@ -0,0 +1,19 @@
+name: Fuzz
+
+on: [push, pull_request, workflow_dispatch]
+
+jobs:
+  test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Install
+        run: |
+          python -m pip install -U pip
+          python -m pip install .
+        env:
+          CFLAGS: '-DDEBUG'
+
+      - name: Fuzz
+        run: python tests/fuzz.py --seed=0:1000

.github/workflows/test.yml

@@ -31,10 +31,18 @@ jobs:
           python -m pip install -U pip
           python -m pip install -U pytest
           python -m pip install .
+        env:
+          CFLAGS: '-DDEBUG'
 
       - name: Tests
         run: |
-          pytest
+          python -m pytest --capture=no
+
+      - name: Test without debug mode
+        run: |
+          git clean -Xfd
+          python -m pip install --force-reinstall .
+          python -m pytest
 
       - name: Test with coverage
         if: ${{ startsWith(matrix.os, 'ubuntu') && matrix.python-version == '3.9' }}

lib/ultrajson.h

@@ -56,9 +56,6 @@ tree doesn't have cyclic references.
 #include <stdio.h>
 #include <wchar.h>
 
-// Don't output any extra whitespaces when encoding
-#define JSON_NO_EXTRA_WHITESPACE
-
 // Max decimals to encode double floating point numbers with
 #ifndef JSON_DOUBLE_MAX_DECIMALS
     #define JSON_DOUBLE_MAX_DECIMALS 15

lib/ultrajsonenc.c

@@ -41,6 +41,7 @@ Numeric decoder derived from from TCL library
 #include <assert.h>
 #include <string.h>
 #include <stdlib.h>
+#include <stddef.h>
 #include <math.h>
 
 #include <float.h>
@@ -113,14 +114,25 @@ FIXME: Keep track of how big these get across several encoder calls and try to m
 That way we won't run our head into the wall each call */
 static void Buffer_Realloc (JSONObjectEncoder *enc, size_t cbNeeded)
 {
+  size_t free_space = enc->end - enc->offset;
+  if (free_space >= cbNeeded)
+  {
+    return;
+  }
   size_t curSize = enc->end - enc->start;
-  size_t newSize = curSize * 2;
+  size_t newSize = curSize;
   size_t offset = enc->offset - enc->start;
 
+#ifdef DEBUG
+  // In debug mode, allocate only what is requested so that any miscalculation
+  // shows up plainly as a crash.
+  newSize = (enc->offset - enc->start) + cbNeeded;
+#else
   while (newSize < curSize + cbNeeded)
   {
     newSize *= 2;
   }
+#endif
 
   if (enc->heap)
   {
@@ -147,6 +159,12 @@ static void Buffer_Realloc (JSONObjectEncoder *enc, size_t cbNeeded)
   enc->end = enc->start + newSize;
 }
 
+#define Buffer_Reserve(__enc, __len) \
+    if ( (size_t) ((__enc)->end - (__enc)->offset) < (size_t) (__len))  \
+    {   \
+      Buffer_Realloc((__enc), (__len));\
+    }   \
+
 static FASTCALL_ATTR INLINE_PREFIX void FASTCALL_MSVC Buffer_AppendShortHexUnchecked (char *outputOffset, unsigned short value)
 {
   *(outputOffset++) = g_hexChars[(value & 0xf000) >> 12];
@@ -266,6 +284,13 @@ static int Buffer_EscapeStringValidated (JSOBJ obj, JSONObjectEncoder *enc, cons
 
   for (;;)
   {
+#ifdef DEBUG
+  // 6 is the maximum length of a single character (cf. RESERVE_STRING).
+  if ((io < end) && (enc->end - of < 6)) {
+    fprintf(stderr, "Ran out of buffer space during Buffer_EscapeStringValidated()\n");
+    abort();
+  }
+#endif
     JSUINT8 utflen = g_asciiOutputTable[(unsigned char) *io];
 
     switch (utflen)
@@ -487,15 +512,28 @@ static int Buffer_EscapeStringValidated (JSOBJ obj, JSONObjectEncoder *enc, cons
   }
 }
 
-#define Buffer_Reserve(__enc, __len) \
-    if ( (size_t) ((__enc)->end - (__enc)->offset) < (size_t) (__len))  \
-    {   \
-      Buffer_Realloc((__enc), (__len));\
-    }   \
-
 
-#define Buffer_AppendCharUnchecked(__enc, __chr) \
-                *((__enc)->offset++) = __chr; \
+static FASTCALL_ATTR INLINE_PREFIX void FASTCALL_MSVC Buffer_AppendCharUnchecked(JSONObjectEncoder *enc, char chr)
+{
+#ifdef DEBUG
+  if (enc->end <= enc->offset)
+  {
+    fprintf(stderr, "Overflow writing byte %d '%c'. The last few characters were:\n'''", chr, chr);
+    char * recent = enc->offset - 1000;
+    if (enc->start > recent)
+    {
+      recent = enc->start;
+    }
+    for (; recent < enc->offset; recent++)
+    {
+      fprintf(stderr, "%c", *recent);
+    }
+    fprintf(stderr, "'''\n");
+    abort();
+  }
+#endif
+  *(enc->offset++) = chr;
+}
 
 static FASTCALL_ATTR INLINE_PREFIX void FASTCALL_MSVC strreverse(char* begin, char* end)
 {
@@ -588,21 +626,15 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
     return;
   }
 
-  /*
-  This reservation must hold
-
-  length of _name as encoded worst case +
-  maxLength of double to string OR maxLength of JSLONG to string
-  */
-
-  Buffer_Reserve(enc, 256 + RESERVE_STRING(cbName));
   if (enc->errorMsg)
   {
     return;
   }
 
   if (name)
   {
+    // 2 extra for the colon and optional space after it
+    Buffer_Reserve(enc, RESERVE_STRING(cbName) + 2);
     Buffer_AppendCharUnchecked(enc, '\"');
 
     if (enc->forceASCII)
@@ -623,19 +655,26 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
     Buffer_AppendCharUnchecked(enc, '\"');
 
     Buffer_AppendCharUnchecked (enc, ':');
-#ifdef JSON_NO_EXTRA_WHITESPACE
     if (enc->indent)
     {
       Buffer_AppendCharUnchecked (enc, ' ');
     }
-#else
-    Buffer_AppendCharUnchecked (enc, ' ');
-#endif
   }
 
   tc.encoder_prv = enc->prv;
   enc->beginTypeContext(obj, &tc, enc);
 
+  /*
+  This reservation covers any additions on non-variable parts below, specifically:
+  - Opening brackets for JT_ARRAY and JT_OBJECT
+  - Number representation for JT_LONG, JT_ULONG, JT_INT, and JT_DOUBLE
+  - Constant value for JT_TRUE, JT_FALSE, JT_NULL
+
+  The length of 128 is the worst case length of the Buffer_AppendDoubleDconv addition.
+  The other types above all have smaller representations.
+  */
+  Buffer_Reserve (enc, 128);
+
   switch (tc.type)
   {
     case JT_INVALID:
@@ -658,12 +697,12 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
 
       while (enc->iterNext(obj, &tc))
       {
+        // The extra 2 bytes cover the comma and (optional) newline.
+        Buffer_Reserve (enc, enc->indent * (enc->level + 1) + 2);
+
         if (count > 0)
         {
           Buffer_AppendCharUnchecked (enc, ',');
-#ifndef JSON_NO_EXTRA_WHITESPACE
-          Buffer_AppendCharUnchecked (enc, ' ');
-#endif
         }
         Buffer_AppendIndentNewlineUnchecked (enc);
 
@@ -685,9 +724,12 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
       enc->iterEnd(obj, &tc);
 
       if (count > 0) {
+        // Reserve space for the indentation plus the newline.
+        Buffer_Reserve (enc, enc->indent * enc->level + 1);
         Buffer_AppendIndentNewlineUnchecked (enc);
         Buffer_AppendIndentUnchecked (enc, enc->level);
       }
+      Buffer_Reserve (enc, 1);
       Buffer_AppendCharUnchecked (enc, ']');
       break;
     }
@@ -700,6 +742,9 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
 
       while ((res = enc->iterNext(obj, &tc)))
       {
+        // The extra 2 bytes cover the comma and optional newline.
+        Buffer_Reserve (enc, enc->indent * (enc->level + 1) + 2);
+
         if(res < 0)
         {
           enc->iterEnd(obj, &tc);
@@ -711,9 +756,6 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
         if (count > 0)
         {
           Buffer_AppendCharUnchecked (enc, ',');
-#ifndef JSON_NO_EXTRA_WHITESPACE
-          Buffer_AppendCharUnchecked (enc, ' ');
-#endif
         }
         Buffer_AppendIndentNewlineUnchecked (enc);
 
@@ -736,9 +778,11 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
       enc->iterEnd(obj, &tc);
 
       if (count > 0) {
+        Buffer_Reserve (enc, enc->indent * enc->level + 1);
         Buffer_AppendIndentNewlineUnchecked (enc);
         Buffer_AppendIndentUnchecked (enc, enc->level);
       }
+      Buffer_Reserve (enc, 1);
       Buffer_AppendCharUnchecked (enc, '}');
       break;
     }
@@ -843,7 +887,7 @@ static void encode(JSOBJ obj, JSONObjectEncoder *enc, const char *name, size_t c
         return;
       }
 
-      Buffer_Reserve(enc, RESERVE_STRING(szlen));
+      Buffer_Reserve(enc, szlen);
       if (enc->errorMsg)
       {
         enc->endTypeContext(obj, &tc);