Fixed segmentation fault in DynamicJsonBuffer when memory allocation fails (issue #92)

.clang-format

@@ -0,0 +1,3 @@
+# http://clang.llvm.org/docs/ClangFormatStyleOptions.html
+
+BasedOnStyle: Google

.gitattributes

@@ -0,0 +1 @@
+*.sh text eol=lf

CHANGELOG.md

@@ -1,6 +1,25 @@
 Arduino JSON: change log
 ========================
 
+v4.6
+----
+
+* Fixed segmentation fault in `DynamicJsonBuffer` when memory allocation fails (issue #92)
+
+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.
+
+Special thanks to [Giancarlo Canales Barreto](https://github.com/gcanalesb) for finding this nasty bug.
+
+v4.4
+----
+
+* Added `JsonArray::measureLength()` and `JsonObject::measureLength()` (issue #75)
+
 v4.3
 ----
 

include/ArduinoJson/DynamicJsonBuffer.hpp

@@ -6,41 +6,12 @@
 
 #pragma once
 
-#include "JsonBuffer.hpp"
+#include "Internals/BlockJsonBuffer.hpp"
-
-#include <stdlib.h>
 
 namespace ArduinoJson {
-
-// Forward declaration
-namespace Internals {
-struct DynamicJsonBufferBlock;
-}
-
 // Implements a JsonBuffer with dynamic memory allocation.
 // You are strongly encouraged to consider using StaticJsonBuffer which is much
 // more suitable for embedded systems.
-class DynamicJsonBuffer : public JsonBuffer {
+typedef Internals::BlockJsonBuffer<Internals::DefaultAllocator>
- public:
+    DynamicJsonBuffer;
-  DynamicJsonBuffer();
-  ~DynamicJsonBuffer();
-
-  size_t size() const;
-
- protected:
-  virtual void* alloc(size_t bytes);
-
- private:
-  typedef Internals::DynamicJsonBufferBlock Block;
-
-  static const size_t FIRST_BLOCK_CAPACITY = 32;
-
-  static Block* createBlock(size_t capacity);
-
-  inline bool canAllocInHead(size_t bytes) const;
-  inline void* allocInHead(size_t bytes);
-  inline void addNewBlock();
-
-  Block* _head;
-};
 }

include/ArduinoJson/Internals/BlockJsonBuffer.hpp

@@ -0,0 +1,93 @@
+// Copyright Benoit Blanchon 2014-2015
+// MIT License
+//
+// Arduino JSON library
+// https://github.com/bblanchon/ArduinoJson
+
+#pragma once
+
+#include "../JsonBuffer.hpp"
+
+#include <stdlib.h>
+
+namespace ArduinoJson {
+namespace Internals {
+class DefaultAllocator {
+ public:
+  void* allocate(size_t size) { return malloc(size); }
+  void deallocate(void* pointer) { free(pointer); }
+};
+
+template <typename TAllocator>
+class BlockJsonBuffer : public JsonBuffer {
+  struct Block;
+  struct EmptyBlock {
+    Block* next;
+    size_t capacity;
+    size_t size;
+  };
+  struct Block : EmptyBlock {
+    uint8_t data[1];
+  };
+
+ public:
+  BlockJsonBuffer() : _head(NULL) {}
+
+  ~BlockJsonBuffer() {
+    Block* currentBlock = _head;
+
+    while (currentBlock != NULL) {
+      Block* nextBlock = currentBlock->next;
+      _allocator.deallocate(currentBlock);
+      currentBlock = nextBlock;
+    }
+  }
+
+  size_t size() const {
+    size_t total = 0;
+    for (const Block* b = _head; b; b = b->next) total += b->size;
+    return total;
+  }
+
+ protected:
+  virtual void* alloc(size_t bytes) {
+    return canAllocInHead(bytes) ? allocInHead(bytes) : allocInNewBlock(bytes);
+  }
+
+ private:
+  static const size_t FIRST_BLOCK_CAPACITY = 32;
+
+  bool canAllocInHead(size_t bytes) const {
+    return _head != NULL && _head->size + bytes <= _head->capacity;
+  }
+
+  void* allocInHead(size_t bytes) {
+    void* p = _head->data + _head->size;
+    _head->size += bytes;
+    return p;
+  }
+
+  void* allocInNewBlock(size_t bytes) {
+    size_t capacity = FIRST_BLOCK_CAPACITY;
+    if (_head != NULL) capacity = _head->capacity * 2;
+    if (bytes > capacity) capacity = bytes;
+    if (!addNewBlock(capacity)) return NULL;
+    return allocInHead(bytes);
+  }
+
+  bool addNewBlock(size_t capacity) {
+    size_t size = sizeof(EmptyBlock) + capacity;
+    Block* block = static_cast<Block*>(_allocator.allocate(size));
+    if (block == NULL) return false;
+    block->capacity = capacity;
+    block->size = 0;
+    block->next = _head;
+    _head = block;
+    return true;
+  }
+
+  Block* _head;
+  TAllocator _allocator;
+};
+}
+}

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/build-arduino-package.sh

@@ -1,6 +1,5 @@
 #!/bin/bash
 
-ZIP="C:\Program Files\7-Zip\7z.exe"
 TAG=$(git describe)
 OUTPUT="ArduinoJson-$TAG.zip"
 
@@ -10,7 +9,7 @@ cd ../..
 rm -f $OUTPUT
 
 # create zip
-"$ZIP" a $OUTPUT \
+7z a $OUTPUT \
 	ArduinoJson/CHANGELOG.md \
 	ArduinoJson/examples \
 	ArduinoJson/include \

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/DynamicJsonBuffer.cpp

@@ -1,79 +0,0 @@
-// Copyright Benoit Blanchon 2014-2015
-// MIT License
-//
-// Arduino JSON library
-// https://github.com/bblanchon/ArduinoJson
-
-#include "../include/ArduinoJson/DynamicJsonBuffer.hpp"
-
-namespace ArduinoJson {
-namespace Internals {
-struct DynamicJsonBufferBlockWithoutData {
-  DynamicJsonBufferBlock* next;
-  size_t capacity;
-  size_t size;
-};
-
-
-struct DynamicJsonBufferBlock : DynamicJsonBufferBlockWithoutData {
-  uint8_t data[1];
-};
-}
-}
-
-using namespace ArduinoJson;
-using namespace ArduinoJson::Internals;
-
-DynamicJsonBuffer::DynamicJsonBuffer() {
-  _head = createBlock(FIRST_BLOCK_CAPACITY);
-}
-
-DynamicJsonBuffer::~DynamicJsonBuffer() {
-  Block* currentBlock = _head;
-
-  while (currentBlock != NULL) {
-    Block* nextBlock = currentBlock->next;
-    free(currentBlock);
-    currentBlock = nextBlock;
-  }
-}
-
-size_t DynamicJsonBuffer::size() const {
-  size_t total = 0;
-
-  for (const Block* b = _head; b != NULL; b = b->next) {
-    total += b->size;
-  }
-
-  return total;
-}
-
-void* DynamicJsonBuffer::alloc(size_t bytes) {
-  if (!canAllocInHead(bytes)) addNewBlock();
-  return allocInHead(bytes);
-}
-
-bool DynamicJsonBuffer::canAllocInHead(size_t bytes) const {
-  return _head->size + bytes <= _head->capacity;
-}
-
-void* DynamicJsonBuffer::allocInHead(size_t bytes) {
-  void* p = _head->data + _head->size;
-  _head->size += bytes;
-  return p;
-}
-
-void DynamicJsonBuffer::addNewBlock() {
-  Block* block = createBlock(_head->capacity * 2);
-  block->next = _head;
-  _head = block;
-}
-
-DynamicJsonBuffer::Block* DynamicJsonBuffer::createBlock(size_t capacity) {
-  size_t blkSize = sizeof(DynamicJsonBufferBlockWithoutData) + capacity;
-  Block* block = static_cast<Block*>(malloc(blkSize));
-  block->capacity = capacity;
-  block->size = 0;
-  block->next = NULL;
-  return block;
-}

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') {
+    if (c == '\0') goto ERROR_CLOSING_QUOTE_MISSING;
-      // premature ending
-      return NULL;
-    }
 
-    if (c == stopChar) {
+    if (c == stopChar) goto SUCCESS;
-      // closing quote
-      break;
-    }
 
     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/DynamicJsonBuffer_NoMemory_Tests.cpp

@@ -0,0 +1,37 @@
+// Copyright Benoit Blanchon 2014-2015
+// MIT License
+//
+// Arduino JSON library
+// https://github.com/bblanchon/ArduinoJson
+
+#include <gtest/gtest.h>
+#include <ArduinoJson.h>
+
+class DynamicJsonBuffer_NoMemory_Tests : public ::testing::Test {
+  class NoMemoryAllocator {
+   public:
+    void* allocate(size_t) { return NULL; }
+    void deallocate(void*) {}
+  };
+
+ protected:
+  Internals::BlockJsonBuffer<NoMemoryAllocator> _jsonBuffer;
+};
+
+TEST_F(DynamicJsonBuffer_NoMemory_Tests, CreateArray) {
+  ASSERT_FALSE(_jsonBuffer.createArray().success());
+}
+
+TEST_F(DynamicJsonBuffer_NoMemory_Tests, CreateObject) {
+  ASSERT_FALSE(_jsonBuffer.createObject().success());
+}
+
+TEST_F(DynamicJsonBuffer_NoMemory_Tests, ParseArray) {
+  char json[] = "[]";
+  ASSERT_FALSE(_jsonBuffer.parseArray(json).success());
+}
+
+TEST_F(DynamicJsonBuffer_NoMemory_Tests, ParseObject) {
+  char json[] = "{}";
+  ASSERT_FALSE(_jsonBuffer.parseObject(json).success());
+}

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));
+    char actual[256];
-    EXPECT_STREQ(expected, _buffer);
-    EXPECT_EQ(strlen(expected), n);
-  }
 
- private:
+    size_t actualLen = array.prettyPrintTo(actual, sizeof(actual));
-  char _buffer[256];
+    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));
+    char buffer[256];
-    EXPECT_STREQ(expected, buffer);
-    EXPECT_EQ(strlen(expected), n);
-  }
 
- private:
+    size_t actualLen = _object.prettyPrintTo(buffer, sizeof(buffer));
-  char buffer[256];
+    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);
+}