Commit 56df2068 authored by Christopher Dunn's avatar Christopher Dunn
Browse files

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.
Showing with 12 additions and 0 deletions
+12 -0
......@@ -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_ = &currentValue();
}
--stackDepth_g;
return successful;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment