From 56df2068470241f9043b676bfae415ed62a0c172 Mon Sep 17 00:00:00 2001 From: Christopher Dunn Date: Wed, 11 Feb 2015 10:20:53 -0600 Subject: [PATCH] limit stackDepth for old (deprecated) Json::Reader too This is an improper solution. If multiple Readers exist, then the effect stackLimit is reduced because of side-effects. But our options are limited. We need to address the security hole without breaking binary-compatibility. However, this is not likely to cause any practical problems because: * Anyone using `operator>>(istream, Json::Value)` will be using the new code already * Multiple Readers are uncommon. * The stackLimit is quite high. * Deeply nested JSON probably would have hit the system limits anyway. --- src/lib_json/json_reader.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/lib_json/json_reader.cpp b/src/lib_json/json_reader.cpp index 6cf23e3..1e7db68 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; }