From 9383e1ca4f3370181dec02ce2e270e35afc3e1a8 Mon Sep 17 00:00:00 2001 From: Kyle Nekritz Date: Wed, 27 Apr 2016 09:42:56 -0700 Subject: [PATCH] Add recursion limit to folly::parseJson. Summary: Without this, malicious inputs can crash anything using folly::parseJson. Reviewed By: yfeldblum Differential Revision: D3219036 fb-gh-sync-id: 3604a060170c0201473c420035b21b018383789c fbshipit-source-id: 3604a060170c0201473c420035b21b018383789c --- folly/json.cpp | 30 +++++++++++++++++++++++++++++- folly/json.h | 31 +++++++++++++++++-------------- folly/test/JsonTest.cpp | 16 ++++++++++++++++ 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/folly/json.cpp b/folly/json.cpp index 9d2a5365..86600fd5 100644 --- a/folly/json.cpp +++ b/folly/json.cpp @@ -379,7 +379,18 @@ struct Input { return opts_; } -private: + void incrementRecursionLevel() { + if (currentRecursionLevel_ > opts_.recursion_limit) { + error("recursion limit exceeded"); + } + currentRecursionLevel_++; + } + + void decrementRecursionLevel() { + currentRecursionLevel_--; + } + + private: void storeCurrent() { current_ = range_.empty() ? EOF : range_.front(); } @@ -389,6 +400,21 @@ private: json::serialization_opts const& opts_; unsigned lineNum_; int current_; + unsigned int currentRecursionLevel_{0}; +}; + +class RecursionGuard { + public: + explicit RecursionGuard(Input& in) : in_(in) { + in_.incrementRecursionLevel(); + } + + ~RecursionGuard() { + in_.decrementRecursionLevel(); + } + + private: + Input& in_; }; dynamic parseValue(Input& in); @@ -627,6 +653,8 @@ std::string parseString(Input& in) { } dynamic parseValue(Input& in) { + RecursionGuard guard(in); + in.skipWhitespace(); return *in == '[' ? parseArray(in) : *in == '{' ? parseObject(in) : diff --git a/folly/json.h b/folly/json.h index 272d610b..15835247 100644 --- a/folly/json.h +++ b/folly/json.h @@ -54,20 +54,20 @@ namespace json { struct serialization_opts { explicit serialization_opts() - : allow_non_string_keys(false) - , javascript_safe(false) - , pretty_formatting(false) - , encode_non_ascii(false) - , validate_utf8(false) - , allow_trailing_comma(false) - , sort_keys(false) - , skip_invalid_utf8(false) - , allow_nan_inf(false) - , double_mode(double_conversion::DoubleToStringConverter::SHORTEST) - , double_num_digits(0) // ignored when mode is SHORTEST - , double_fallback(false) - , parse_numbers_as_strings(false) - {} + : allow_non_string_keys(false), + javascript_safe(false), + pretty_formatting(false), + encode_non_ascii(false), + validate_utf8(false), + allow_trailing_comma(false), + sort_keys(false), + skip_invalid_utf8(false), + allow_nan_inf(false), + double_mode(double_conversion::DoubleToStringConverter::SHORTEST), + double_num_digits(0), // ignored when mode is SHORTEST + double_fallback(false), + parse_numbers_as_strings(false), + recursion_limit(100) {} // If true, keys in an object can be non-strings. (In strict // JSON, object keys must be strings.) This is used by dynamic's @@ -115,6 +115,9 @@ namespace json { // Do not parse numbers. Instead, store them as strings and leave the // conversion up to the user. bool parse_numbers_as_strings; + + // Recursion limit when parsing. + unsigned int recursion_limit; }; /* diff --git a/folly/test/JsonTest.cpp b/folly/test/JsonTest.cpp index 64430ff5..a9bca3ba 100644 --- a/folly/test/JsonTest.cpp +++ b/folly/test/JsonTest.cpp @@ -517,3 +517,19 @@ TEST(Json, PrintTo) { PrintTo(value, &oss); EXPECT_EQ(expected, oss.str()); } + +TEST(Json, RecursionLimit) { + std::string in; + for (int i = 0; i < 1000; i++) { + in.append("{\"x\":"); + } + in.append("\"hi\""); + for (int i = 0; i < 1000; i++) { + in.append("}"); + } + EXPECT_ANY_THROW(parseJson(in)); + + folly::json::serialization_opts opts_high_recursion_limit; + opts_high_recursion_limit.recursion_limit = 10000; + parseJson(in, opts_high_recursion_limit); +} -- 2.34.1