Fix buffer overflow (pull request #81)

CHANGELOG.md

@@ -1,6 +1,18 @@
 Arduino JSON: change log
 ========================
 
+v4.5
+----
+
+* Fixed buffer overflow when input contains a backslash followed by a terminator (issue #81)
+
+**Upgrading is recommended** since previous versions contain a potential security risk.
+
+v4.4
+----
+
+* Added `JsonArray::measureLength()` and `JsonObject::measureLength()` (issue #75)
+
 v4.3
 ----
 

include/ArduinoJson/Internals/DummyPrint.hpp

@@ -0,0 +1,20 @@
+// Copyright Benoit Blanchon 2014-2015
+// MIT License
+//
+// Arduino JSON library
+// https://github.com/bblanchon/ArduinoJson
+
+#pragma once
+
+#include "../Arduino/Print.hpp"
+
+namespace ArduinoJson {
+namespace Internals {
+
+// A dummy Print implementation used in JsonPrintable::measureLength()
+class DummyPrint : public Print {
+ public:
+  virtual size_t write(uint8_t) { return 1; }
+};
+}
+}

include/ArduinoJson/Internals/JsonPrintable.hpp

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include "DummyPrint.hpp"
 #include "IndentedPrint.hpp"
 #include "JsonWriter.hpp"
 #include "Prettyfier.hpp"
@@ -47,6 +48,16 @@ class JsonPrintable {
     return prettyPrintTo(indentedPrint);
   }
 
+  size_t measureLength() const {
+    DummyPrint dp;
+    return printTo(dp);
+  }
+
+  size_t measurePrettyLength() const {
+    DummyPrint dp;
+    return prettyPrintTo(dp);
+  }
+
  private:
   const T &downcast() const { return *static_cast<const T *>(this); }
 };

scripts/create-build-envs.sh

@@ -20,10 +20,10 @@ build-env()
 if [[ $(uname) == MINGW* ]]
 then
 	build-env "Make" "MinGW Makefiles"
-	build-env "SublimeText" "Sublime Text 2 - MinGW Makefiles"
+	build-env "SublimeText" "Sublime Text 2 - Ninja"
 	build-env "VisualStudio" "Visual Studio 12 2013"
 else
-	build-env "SublimeText" "Sublime Text 2 - Unix Makefiles"
+	build-env "SublimeText" "Sublime Text 2 - Ninja"
 	build-env "Make" "Unix Makefiles"
 	build-env "Xcode" "Xcode"
 fi

src/Internals/QuotedString.cpp

@@ -58,46 +58,44 @@ static char unescapeChar(char c) {
 static inline bool isQuote(char c) { return c == '\"' || c == '\''; }
 
 char *QuotedString::extractFrom(char *input, char **endPtr) {
-  char firstChar = *input;
-
-  if (!isQuote(firstChar)) {
-    // must start with a quote
-    return NULL;
-  }
-
-  char stopChar = firstChar;  // closing quote is the same as opening quote
-
   char *startPtr = input + 1;  // skip the quote
   char *readPtr = startPtr;
   char *writePtr = startPtr;
   char c;
 
+  char firstChar = *input;
+  char stopChar = firstChar;  // closing quote is the same as opening quote
+
+  if (!isQuote(firstChar)) goto ERROR_OPENING_QUOTE_MISSING;
+
   for (;;) {
     c = *readPtr++;
 
-    if (c == '\0') {
-      // premature ending
-      return NULL;
-    }
+    if (c == '\0') goto ERROR_CLOSING_QUOTE_MISSING;
 
-    if (c == stopChar) {
-      // closing quote
-      break;
-    }
+    if (c == stopChar) goto SUCCESS;
 
     if (c == '\\') {
       // replace char
       c = unescapeChar(*readPtr++);
+      if (c == '\0') goto ERROR_ESCAPE_SEQUENCE_INTERRUPTED;
     }
 
     *writePtr++ = c;
   }
 
+SUCCESS:
   // end the string here
   *writePtr = '\0';
 
   // update end ptr
   *endPtr = readPtr;
 
+  // return pointer to unquoted string
   return startPtr;
+
+ERROR_OPENING_QUOTE_MISSING:
+ERROR_CLOSING_QUOTE_MISSING:
+ERROR_ESCAPE_SEQUENCE_INTERRUPTED:
+  return NULL;
 }

test/JsonArray_PrettyPrintTo_Tests.cpp

@@ -16,13 +16,15 @@ class JsonArray_PrettyPrintTo_Tests : public testing::Test {
   JsonArray& array;
 
   void outputMustBe(const char* expected) {
-    size_t n = array.prettyPrintTo(_buffer, sizeof(_buffer));
-    EXPECT_STREQ(expected, _buffer);
-    EXPECT_EQ(strlen(expected), n);
-  }
+    char actual[256];
 
- private:
-  char _buffer[256];
+    size_t actualLen = array.prettyPrintTo(actual, sizeof(actual));
+    size_t measuredLen = array.measurePrettyLength();
+
+    EXPECT_STREQ(expected, actual);
+    EXPECT_EQ(strlen(expected), actualLen);
+    EXPECT_EQ(strlen(expected), measuredLen);
+  }
 };
 
 TEST_F(JsonArray_PrettyPrintTo_Tests, Empty) { outputMustBe("[]"); }

test/JsonArray_PrintTo_Tests.cpp

@@ -16,9 +16,12 @@ class JsonArray_PrintTo_Tests : public testing::Test {
   JsonArray &array;
 
   void outputMustBe(const char *expected) {
-    size_t n = array.printTo(buffer, sizeof(buffer));
+    size_t actualLen = array.printTo(buffer, sizeof(buffer));
+    size_t measuredLen = array.measureLength();
+
     EXPECT_STREQ(expected, buffer);
-    EXPECT_EQ(strlen(expected), n);
+    EXPECT_EQ(strlen(expected), actualLen);
+    EXPECT_EQ(strlen(expected), measuredLen);
   }
 
  private:

test/JsonObject_PrettyPrintTo_Tests.cpp

@@ -16,13 +16,15 @@ class JsonObject_PrettyPrintTo_Tests : public testing::Test {
   JsonObject &_object;
 
   void outputMustBe(const char *expected) {
-    size_t n = _object.prettyPrintTo(buffer, sizeof(buffer));
-    EXPECT_STREQ(expected, buffer);
-    EXPECT_EQ(strlen(expected), n);
-  }
+    char buffer[256];
 
- private:
-  char buffer[256];
+    size_t actualLen = _object.prettyPrintTo(buffer, sizeof(buffer));
+    size_t measuredLen = _object.measurePrettyLength();
+
+    EXPECT_STREQ(expected, buffer);
+    EXPECT_EQ(strlen(expected), actualLen);
+    EXPECT_EQ(strlen(expected), measuredLen);
+  }
 };
 
 TEST_F(JsonObject_PrettyPrintTo_Tests, EmptyObject) { outputMustBe("{}"); }

test/JsonObject_PrintTo_Tests.cpp

@@ -16,10 +16,12 @@ class JsonObject_PrintTo_Tests : public testing::Test {
  protected:
   void outputMustBe(const char *expected) {
     char actual[256];
-    int result = object.printTo(actual, sizeof(actual));
+    size_t actualLen = object.printTo(actual, sizeof(actual));
+    size_t measuredLen = object.measureLength();
 
     EXPECT_STREQ(expected, actual);
-    EXPECT_EQ(strlen(expected), result);
+    EXPECT_EQ(strlen(expected), actualLen);
+    EXPECT_EQ(strlen(expected), measuredLen);
   }
 
   StaticJsonBuffer<JSON_OBJECT_SIZE(2)> json;

test/QuotedString_ExtractFrom_Tests.cpp

@@ -16,6 +16,11 @@ class QuotedString_ExtractFrom_Tests : public testing::Test {
     _result = QuotedString::extractFrom(_jsonString, &_trailing);
   }
 
+  void whenInputIs(const char *json, size_t len) {
+    memcpy(_jsonString, json, len);
+    _result = QuotedString::extractFrom(_jsonString, &_trailing);
+  }
+
   void resultMustBe(const char *expected) { EXPECT_STREQ(expected, _result); }
 
   void trailingMustBe(const char *expected) {
@@ -134,3 +139,8 @@ TEST_F(QuotedString_ExtractFrom_Tests, AllEscapedCharsTogether) {
   whenInputIs("\"1\\\"2\\\\3\\/4\\b5\\f6\\n7\\r8\\t9\"");
   resultMustBe("1\"2\\3/4\b5\f6\n7\r8\t9");
 }
+
+TEST_F(QuotedString_ExtractFrom_Tests, UnterminatedEscapeSequence) {
+  whenInputIs("\"\\\0\"", 4);
+  resultMustBe(0);
+}