From 181771b71296ffc242a4301a472f5a7ad5f6cc76 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 3 Jun 2015 09:42:03 +0200 Subject: [PATCH] json: fail read_string if string contains trailing garbage Change `read_string` to fail when not the entire input has been consumed. This avoids unexpected, even dangerous behavior (fixes #6223). The new JSON parser adapted in #6121 also solves this problem so in master this is a temporary fix, but should be backported to older releases. Also adds tests for the new behavior. Github-Pull: #6226 Rebased-From: 4e157fc60dae5ca69933ea4c1585a2a078b4d957 --- src/json/json_spirit_reader_template.h | 6 +++--- src/test/rpc_tests.cpp | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/json/json_spirit_reader_template.h b/src/json/json_spirit_reader_template.h index 46f5892f62..47e3c1ca84 100644 --- a/src/json/json_spirit_reader_template.h +++ b/src/json/json_spirit_reader_template.h @@ -521,12 +521,11 @@ namespace json_spirit const spirit_namespace::parse_info< Iter_type > info = spirit_namespace::parse( begin, end, - Json_grammer< Value_type, Iter_type >( semantic_actions ), + Json_grammer< Value_type, Iter_type >( semantic_actions ) >> spirit_namespace::end_p, spirit_namespace::space_p ); if( !info.hit ) { - assert( false ); // in theory exception should already have been thrown throw_error( info.stop, "error" ); } @@ -570,7 +569,8 @@ namespace json_spirit { typename String_type::const_iterator begin = s.begin(); - return read_range( begin, s.end(), value ); + bool success = read_range( begin, s.end(), value ); + return success && begin == s.end(); } template< class Istream_type > diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index d5475a92bf..aed6e3ac21 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -139,6 +139,24 @@ BOOST_AUTO_TEST_CASE(rpc_parse_monetary_values) BOOST_CHECK_EQUAL(AmountFromValue(ValueFromString("20999999.99999999")), 2099999999999999LL); } +BOOST_AUTO_TEST_CASE(json_parse_errors) +{ + Value value; + // Valid + BOOST_CHECK_EQUAL(read_string(std::string("1.0"), value), true); + // Valid, with trailing whitespace + BOOST_CHECK_EQUAL(read_string(std::string("1.0 "), value), true); + // Invalid, initial garbage + BOOST_CHECK_EQUAL(read_string(std::string("[1.0"), value), false); + BOOST_CHECK_EQUAL(read_string(std::string("a1.0"), value), false); + // Invalid, trailing garbage + BOOST_CHECK_EQUAL(read_string(std::string("1.0sds"), value), false); + BOOST_CHECK_EQUAL(read_string(std::string("1.0]"), value), false); + // BTC addresses should fail parsing + BOOST_CHECK_EQUAL(read_string(std::string("175tWpb8K1S7NmH4Zx6rewF9WQrcZv245W"), value), false); + BOOST_CHECK_EQUAL(read_string(std::string("3J98t1WpEZ73CNmQviecrnyiWrnqRhWNL"), value), false); +} + BOOST_AUTO_TEST_CASE(rpc_boostasiotocnetaddr) { // Check IPv4 addresses