json: fail read_string if string contains trailing garbage #6226
Conversation
|
Must be a better way ( Which leads me into a jungle of boost issues... Looks like this problem :https://svn.boost.org/trac/boost/ticket/1048 "Spirit returns full = false in 1.34 if there is trailing spaces in input" OK I followed the instructions there and http://thread.gmane.org/gmane.comp.parsers.spirit.general/9839, this should do it. |
laanwj
force-pushed
the
2015_06_json_trailing_garbage
branch
from
a4993f7 to
46f92fc
Compare
|
Nice! |
|
You should add test cases for the specific case which triggered this in the wild: interpreting an address '1p2pkh...' or '3p2sh...' as a value and getting 1btc / 3btc. |
Change `read_string` to fail when not the entire input has been consumed. This avoids unexpected, even dangerous behavior (fixes bitcoin#6223). The new JSON parser adapted in bitcoin#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.
laanwj
force-pushed
the
2015_06_json_trailing_garbage
branch
from
46f92fc to
4e157fc
Compare
|
@maaku OK, added testcases that parsing bitcoin addresses as JSON will fail |
|
Tested ACK. |
|
Thanks for testing @jonasschnelli |
4e157fc json: fail read_string if string contains trailing garbage (Wladimir J. van der Laan)
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: 4e157fc
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: 4e157fc
|
Backported to 0.11 via a7dcb7eb04357462f65f528a01c37ec59a23ec43 |
Change `read_string` to fail when not the entire input has been consumed. This avoids unexpected, even dangerous behavior (fixes bitcoin#6223). The new JSON parser adapted in bitcoin#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: bitcoin#6226 Rebased-From: 4e157fc (cherry picked from commit 181771b)
3 participants
Change
read_stringto 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.
This now errors as expected:
Needs backport to 0.10 and 0.11. I don't think older versions are affected, as they did manual parsing of bitcoin-cli arguments.
Note: normally I wouldn't change json_spirit directly as it is an upstream library, but it is going to be dropped anyway, so doing the straightforward fix makes sense