From a3425a6306218a36e4486d543d8959ec9e3926ca Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Thu, 6 Nov 2014 10:24:37 +0100 Subject: [PATCH] Added a nesting limit to the parser to prevent stack overflow that could be a security issue --- include/ArduinoJson/Internals/JsonParser.hpp | 4 +- include/ArduinoJson/JsonBuffer.hpp | 6 +- src/Internals/JsonParser.cpp | 5 ++ src/JsonBuffer.cpp | 8 +- test/JsonParser_NestingLimit_Tests.cpp | 88 ++++++++++++++++++++ 5 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 test/JsonParser_NestingLimit_Tests.cpp diff --git a/include/ArduinoJson/Internals/JsonParser.hpp b/include/ArduinoJson/Internals/JsonParser.hpp index 5fc1a58f..2062f37d 100644 --- a/include/ArduinoJson/Internals/JsonParser.hpp +++ b/include/ArduinoJson/Internals/JsonParser.hpp @@ -13,7 +13,8 @@ namespace Internals { class JsonParser { public: - JsonParser(JsonBuffer *buffer, char *json) : _buffer(buffer), _ptr(json) {} + JsonParser(JsonBuffer *buffer, char *json, uint8_t nestingLimit) + : _buffer(buffer), _ptr(json), _nestingLimit(nestingLimit) {} JsonArray &parseArray(); JsonObject &parseObject(); @@ -33,6 +34,7 @@ class JsonParser { JsonBuffer *_buffer; char *_ptr; + uint8_t _nestingLimit; }; } } diff --git a/include/ArduinoJson/JsonBuffer.hpp b/include/ArduinoJson/JsonBuffer.hpp index fffd7804..5d285563 100644 --- a/include/ArduinoJson/JsonBuffer.hpp +++ b/include/ArduinoJson/JsonBuffer.hpp @@ -19,9 +19,11 @@ class JsonBuffer { JsonArray &createArray(); JsonObject &createObject(); - JsonArray &parseArray(char *json); - JsonObject &parseObject(char *json); + JsonArray &parseArray(char *json, uint8_t nestingLimit = DEFAULT_LIMIT); + JsonObject &parseObject(char *json, uint8_t nestingLimit = DEFAULT_LIMIT); virtual void *alloc(size_t size) = 0; + + static const uint8_t DEFAULT_LIMIT = 10; }; } diff --git a/src/Internals/JsonParser.cpp b/src/Internals/JsonParser.cpp index f2e6da02..9085d83c 100644 --- a/src/Internals/JsonParser.cpp +++ b/src/Internals/JsonParser.cpp @@ -39,6 +39,9 @@ bool JsonParser::skip(const char *wordToSkip) { } void JsonParser::parseAnythingTo(JsonVariant &destination) { + if (_nestingLimit == 0) return; + _nestingLimit--; + skipSpaces(); switch (*_ptr) { @@ -79,6 +82,8 @@ void JsonParser::parseAnythingTo(JsonVariant &destination) { destination = parseString(); break; } + + _nestingLimit++; } JsonArray &JsonParser::parseArray() { diff --git a/src/JsonBuffer.cpp b/src/JsonBuffer.cpp index 8f1d6175..49dcf3d9 100644 --- a/src/JsonBuffer.cpp +++ b/src/JsonBuffer.cpp @@ -26,12 +26,12 @@ JsonObject &JsonBuffer::createObject() { return JsonObject::invalid(); } -JsonArray &JsonBuffer::parseArray(char *json) { - JsonParser parser(this, json); +JsonArray &JsonBuffer::parseArray(char *json, uint8_t nestingLimit) { + JsonParser parser(this, json, nestingLimit); return parser.parseArray(); } -JsonObject &JsonBuffer::parseObject(char *json) { - JsonParser parser(this, json); +JsonObject &JsonBuffer::parseObject(char *json, uint8_t nestingLimit) { + JsonParser parser(this, json, nestingLimit); return parser.parseObject(); } diff --git a/test/JsonParser_NestingLimit_Tests.cpp b/test/JsonParser_NestingLimit_Tests.cpp new file mode 100644 index 00000000..a5ddcfc2 --- /dev/null +++ b/test/JsonParser_NestingLimit_Tests.cpp @@ -0,0 +1,88 @@ +// Copyright Benoit Blanchon 2014 +// MIT License +// +// Arduino JSON library +// https://github.com/bblanchon/ArduinoJson + +#include +#include +#include +#include + +using namespace ArduinoJson; + +class JsonParser_NestingLimit_Tests : public testing::Test { + protected: + void whenNestingLimitIs(uint8_t nestingLimit) { + _nestingLimit = nestingLimit; + } + + void parseArrayMustFail(const char *json) { + ASSERT_FALSE(tryParseArray(json)); + } + + void parseArrayMustSucceed(const char *json) { + ASSERT_TRUE(tryParseArray(json)); + } + + void parseObjectMustFail(const char *json) { + ASSERT_FALSE(tryParseObject(json)); + } + + void parseObjectMustSucceed(const char *json) { + ASSERT_TRUE(tryParseObject(json)); + } + + private: + bool tryParseArray(const char *json) { + StaticJsonBuffer<256> buffer; + char s[256]; + strcpy(s, json); + return buffer.parseArray(s, _nestingLimit).success(); + } + + bool tryParseObject(const char *json) { + StaticJsonBuffer<256> buffer; + char s[256]; + strcpy(s, json); + return buffer.parseObject(s, _nestingLimit).success(); + } + + uint8_t _nestingLimit; +}; + +TEST_F(JsonParser_NestingLimit_Tests, ParseArrayWithNestingLimit0) { + whenNestingLimitIs(0); + parseArrayMustSucceed("[]"); + parseArrayMustFail("[[]]"); +} + +TEST_F(JsonParser_NestingLimit_Tests, ParseArrayWithNestingLimit1) { + whenNestingLimitIs(1); + parseArrayMustSucceed("[[]]"); + parseArrayMustFail("[[[]]]"); +} + +TEST_F(JsonParser_NestingLimit_Tests, ParseArrayWithNestingLimit2) { + whenNestingLimitIs(2); + parseArrayMustSucceed("[[[]]]"); + parseArrayMustFail("[[[[]]]]"); +} + +TEST_F(JsonParser_NestingLimit_Tests, ParseObjectWithNestingLimit0) { + whenNestingLimitIs(0); + parseObjectMustSucceed("{}"); + parseObjectMustFail("{\"key\":{}}"); +} + +TEST_F(JsonParser_NestingLimit_Tests, ParseObjectWithNestingLimit1) { + whenNestingLimitIs(1); + parseObjectMustSucceed("{\"key\":{}}"); + parseObjectMustFail("{\"key\":{\"key\":{}}}"); +} + +TEST_F(JsonParser_NestingLimit_Tests, ParseObjectWithNestingLimit2) { + whenNestingLimitIs(2); + parseObjectMustSucceed("{\"key\":{\"key\":{}}}"); + parseObjectMustFail("{\"key\":{\"key\":{\"key\":{}}}}"); +}