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

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