Merge pull request #377 from haberman/json-parse-null

Fixed "NULL + 0" UB in JSON encoder and decoder.

.bazelrc

@@ -8,6 +8,8 @@ build:ubsan --copt=-fsanitize=undefined --linkopt=-fsanitize=undefined --action_
 # Workaround for the fact that Bazel links with $CC, not $CXX
 # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748
 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr
+# Workaround for https://bugs.llvm.org/show_bug.cgi?id=16404
+build:ubsan --linkopt=--rtlib=compiler-rt --linkopt=-lunwind
 
 build:Werror --copt=-Werror
 build:Werror --per_file_copt=json/parser@-Wno-error

tests/BUILD

@@ -97,6 +97,7 @@ cc_test(
         "//:port",
         "//:reflection",
         "//:upb",
+        "//:json",
     ],
 )
 

tests/test_cpp.cc

@@ -12,9 +12,12 @@
 #include <sstream>
 
 #include "tests/test_cpp.upbdefs.h"
+#include "tests/test_cpp.upb.h"
 #include "tests/upb_test.h"
 #include "upb/def.h"
 #include "upb/def.hpp"
+#include "upb/json_decode.h"
+#include "upb/json_encode.h"
 #include "upb/upb.h"
 
 // Must be last.
@@ -102,6 +105,15 @@ void TestInlinedArena() {
 }
 
 void TestDefault() {
+  upb::SymbolTable symtab;
+  upb::Arena arena;
+  upb::MessageDefPtr md(upb_test_TestMessage_getmsgdef(symtab.ptr()));
+  upb_test_TestMessage *msg = upb_test_TestMessage_new(arena.ptr());
+  size_t size = upb_json_encode(msg, md.ptr(), NULL, 0, NULL, 0, NULL);
+  ASSERT(size == 2);  // "{}"
+}
+
+void TestJsonNull() {
   upb::SymbolTable symtab;
   upb::MessageDefPtr md(upb_test_TestMessage_getmsgdef(symtab.ptr()));
   upb::FieldDefPtr i32_f = md.FindFieldByName("i32");

upb/json_decode.c

@@ -1427,6 +1427,9 @@ bool upb_json_decode(const char *buf, size_t size, upb_msg *msg,
                      const upb_msgdef *m, const upb_symtab *any_pool,
                      int options, upb_arena *arena, upb_status *status) {
   jsondec d;
+
+  if (size == 0) return true;
+
   d.ptr = buf;
   d.end = buf + size;
   d.arena = arena;

upb/json_encode.c

@@ -63,8 +63,10 @@ static void jsonenc_putbytes(jsonenc *e, const void *data, size_t len) {
     memcpy(e->ptr, data, len);
     e->ptr += len;
   } else {
-    if (have) memcpy(e->ptr, data, have);
-    e->ptr += have;
+    if (have) {
+      memcpy(e->ptr, data, have);
+      e->ptr += have;
+    }
     e->overflow += (len - have);
   }
 }
@@ -86,7 +88,7 @@ static void jsonenc_printf(jsonenc *e, const char *fmt, ...) {
   if (UPB_LIKELY(have > n)) {
     e->ptr += n;
   } else {
-    e->ptr += have;
+    e->ptr = UPB_PTRADD(e->ptr, have);
     e->overflow += (n - have);
   }
 }
@@ -190,7 +192,7 @@ static void jsonenc_bytes(jsonenc *e, upb_strview str) {
   static const char base64[] =
       "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
   const unsigned char *ptr = (unsigned char*)str.data;
-  const unsigned char *end = ptr + str.size;
+  const unsigned char *end = UPB_PTRADD(ptr, str.size);
   char buf[4];
 
   jsonenc_putstr(e, "\"");
@@ -226,7 +228,7 @@ static void jsonenc_bytes(jsonenc *e, upb_strview str) {
 
 static void jsonenc_stringbody(jsonenc *e, upb_strview str) {
   const char *ptr = str.data;
-  const char *end = ptr + str.size;
+  const char *end = UPB_PTRADD(ptr, str.size);
 
   while (ptr < end) {
     switch (*ptr) {
@@ -698,7 +700,7 @@ size_t upb_json_encode(const upb_msg *msg, const upb_msgdef *m,
 
   e.buf = buf;
   e.ptr = buf;
-  e.end = buf + size;
+  e.end = UPB_PTRADD(buf, size);
   e.overflow = 0;
   e.options = options;
   e.ext_pool = ext_pool;

upb/port_def.inc

@@ -135,6 +135,9 @@
 #define UPB_LONGJMP(buf, val) longjmp(buf, val)
 #endif
 
+/* UPB_PTRADD(ptr, ofs): add pointer while avoiding "NULL + 0" UB */
+#define UPB_PTRADD(ptr, ofs) ((ofs) ? (ptr) + (ofs) : (ptr))
+
 /* Configure whether fasttable is switched on or not. *************************/
 
 #if (defined(__x86_64__) || defined(__aarch64__)) && defined(__GNUC__)

upb/port_undef.inc

@@ -1,24 +1,33 @@
 /* See port_def.inc.  This should #undef all macros #defined there. */
 
-#undef UPB_MAPTYPE_STRING
 #undef UPB_SIZE
 #undef UPB_PTR_AT
 #undef UPB_READ_ONEOF
 #undef UPB_WRITE_ONEOF
+#undef UPB_MAPTYPE_STRING
 #undef UPB_INLINE
 #undef UPB_ALIGN_UP
 #undef UPB_ALIGN_DOWN
 #undef UPB_ALIGN_MALLOC
 #undef UPB_ALIGN_OF
+#undef UPB_LIKELY
+#undef UPB_UNLIKELY
 #undef UPB_FORCEINLINE
 #undef UPB_NOINLINE
 #undef UPB_NORETURN
+#undef UPB_PRINTF
 #undef UPB_MAX
 #undef UPB_MIN
 #undef UPB_UNUSED
 #undef UPB_ASSUME
 #undef UPB_ASSERT
 #undef UPB_UNREACHABLE
+#undef UPB_SETJMP
+#undef UPB_LONGJMP
+#undef UPB_PTRADD
+#undef UPB_FASTTABLE_SUPPORTED
+#undef UPB_FASTTABLE
+#undef UPB_FASTTABLE_INIT
 #undef UPB_POISON_MEMORY_REGION
 #undef UPB_UNPOISON_MEMORY_REGION
 #undef UPB_ASAN

upb/text_encode.c

@@ -28,8 +28,10 @@ static void txtenc_putbytes(txtenc *e, const void *data, size_t len) {
     memcpy(e->ptr, data, len);
     e->ptr += len;
   } else {
-    if (have) memcpy(e->ptr, data, have);
-    e->ptr += have;
+    if (have) {
+      memcpy(e->ptr, data, have);
+      e->ptr += have;
+    }
     e->overflow += (len - have);
   }
 }
@@ -50,7 +52,7 @@ static void txtenc_printf(txtenc *e, const char *fmt, ...) {
   if (UPB_LIKELY(have > n)) {
     e->ptr += n;
   } else {
-    e->ptr += have;
+    e->ptr = UPB_PTRADD(e->ptr, have);
     e->overflow += (n - have);
   }
 }
@@ -408,7 +410,7 @@ size_t upb_text_encode(const upb_msg *msg, const upb_msgdef *m,
 
   e.buf = buf;
   e.ptr = buf;
-  e.end = buf + size;
+  e.end = UPB_PTRADD(buf, size);
   e.overflow = 0;
   e.indent_depth = 0;
   e.options = options;