diff --git a/include/json/reader.h b/include/json/reader.h index 041ae0d37..b5b2e9743 100644 --- a/include/json/reader.h +++ b/include/json/reader.h @@ -310,6 +310,9 @@ class JSON_API CharReaderBuilder : public CharReader::Factory { - true if dropped null placeholders are allowed. (See StreamWriterBuilder.) - "allowNumericKeys": false or true - true if numeric object keys are allowed. + - "stackLimit": integer + - This is a security issue (seg-faults caused by deeply nested JSON), + so the default is low. You can examine 'settings_` yourself to see the defaults. You can also write and read them just like any diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index b4c896587..1e7db6896 100644 --- a/src/lib_json/json_reader.cpp +++ b/src/lib_json/json_reader.cpp @@ -28,6 +28,9 @@ #pragma warning(disable : 4996) #endif +static int const stackLimit_g = 1000; +static int stackDepth_g = 0; // see readValue() + namespace Json { #if __cplusplus >= 201103L @@ -118,6 +121,7 @@ bool Reader::parse(const char* beginDoc, nodes_.pop(); nodes_.push(&root); + stackDepth_g = 0; // Yes, this is bad coding, but options are limited. bool successful = readValue(); Token token; skipCommentTokens(token); @@ -140,6 +144,13 @@ bool Reader::parse(const char* beginDoc, } bool Reader::readValue() { + // This is a non-reentrant way to support a stackLimit. Terrible! + // But this deprecated class has a security problem: Bad input can + // cause a seg-fault. This seems like a fair, binary-compatible way + // to prevent the problem. + if (stackDepth_g >= stackLimit_g) throw std::runtime_error("Exceeded stackLimit in readValue()."); + ++stackDepth_g; + Token token; skipCommentTokens(token); bool successful = true; @@ -211,6 +222,7 @@ bool Reader::readValue() { lastValue_ = ¤tValue(); } + --stackDepth_g; return successful; } @@ -902,7 +914,8 @@ class OurFeatures { bool strictRoot_; bool allowDroppedNullPlaceholders_; bool allowNumericKeys_; -}; // OldFeatures + int stackLimit_; +}; // OurFeatures // exact copy of Implementation of class Features // //////////////////////////////// @@ -1033,7 +1046,9 @@ class OurReader { Location lastValueEnd_; Value* lastValue_; std::string commentsBefore_; - OurFeatures features_; + int stackDepth_; + + OurFeatures const features_; bool collectComments_; }; // OurReader @@ -1064,6 +1079,7 @@ bool OurReader::parse(const char* beginDoc, nodes_.pop(); nodes_.push(&root); + stackDepth_ = 0; bool successful = readValue(); Token token; skipCommentTokens(token); @@ -1086,6 +1102,8 @@ bool OurReader::parse(const char* beginDoc, } bool OurReader::readValue() { + if (stackDepth_ >= features_.stackLimit_) throw std::runtime_error("Exceeded stackLimit in readValue()."); + ++stackDepth_; Token token; skipCommentTokens(token); bool successful = true; @@ -1157,6 +1175,7 @@ bool OurReader::readValue() { lastValue_ = ¤tValue(); } + --stackDepth_; return successful; } @@ -1853,6 +1872,7 @@ CharReader* CharReaderBuilder::newCharReader() const features.strictRoot_ = settings_["strictRoot"].asBool(); features.allowDroppedNullPlaceholders_ = settings_["allowDroppedNullPlaceholders"].asBool(); features.allowNumericKeys_ = settings_["allowNumericKeys"].asBool(); + features.stackLimit_ = settings_["stackLimit"].asInt(); return new OurCharReader(collectComments, features); } static void getValidReaderKeys(std::set* valid_keys) @@ -1863,6 +1883,7 @@ static void getValidReaderKeys(std::set* valid_keys) valid_keys->insert("strictRoot"); valid_keys->insert("allowDroppedNullPlaceholders"); valid_keys->insert("allowNumericKeys"); + valid_keys->insert("stackLimit"); } bool CharReaderBuilder::validate(Json::Value* invalid) const { @@ -1901,6 +1922,7 @@ void CharReaderBuilder::setDefaults(Json::Value* settings) (*settings)["strictRoot"] = false; (*settings)["allowDroppedNullPlaceholders"] = false; (*settings)["allowNumericKeys"] = false; + (*settings)["stackLimit"] = 1000; //! [CharReaderBuilderDefaults] } diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index e4d2dcbc2..30c80e2e8 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -1713,6 +1713,34 @@ JSONTEST_FIXTURE(CharReaderTest, parseWithDetailError) { delete reader; } +JSONTEST_FIXTURE(CharReaderTest, parseWithStackLimit) { + Json::CharReaderBuilder b; + Json::Value root; + char const doc[] = + "{ \"property\" : \"value\" }"; + { + b.settings_["stackLimit"] = 2; + Json::CharReader* reader(b.newCharReader()); + std::string errs; + bool ok = reader->parse( + doc, doc + std::strlen(doc), + &root, &errs); + JSONTEST_ASSERT(ok); + JSONTEST_ASSERT(errs == ""); + JSONTEST_ASSERT_EQUAL("value", root["property"]); + delete reader; + } + { + b.settings_["stackLimit"] = 1; + Json::CharReader* reader(b.newCharReader()); + std::string errs; + JSONTEST_ASSERT_THROWS(reader->parse( + doc, doc + std::strlen(doc), + &root, &errs)); + delete reader; + } +} + int main(int argc, const char* argv[]) { JsonTest::Runner runner; JSONTEST_REGISTER_FIXTURE(runner, ValueTest, checkNormalizeFloatingPointStr); @@ -1749,6 +1777,7 @@ int main(int argc, const char* argv[]) { JSONTEST_REGISTER_FIXTURE(runner, CharReaderTest, parseWithOneError); JSONTEST_REGISTER_FIXTURE(runner, CharReaderTest, parseChineseWithOneError); JSONTEST_REGISTER_FIXTURE(runner, CharReaderTest, parseWithDetailError); + JSONTEST_REGISTER_FIXTURE(runner, CharReaderTest, parseWithStackLimit); JSONTEST_REGISTER_FIXTURE(runner, WriterTest, dropNullPlaceholders); JSONTEST_REGISTER_FIXTURE(runner, StreamWriterTest, dropNullPlaceholders);