RPC validation checks for sendtoaddress #6223

1. 
   btcdrak commented at 0:10 am on June 3, 2015: contributor

   Reported on IRC

   0 <freik> Wow, just accidentally sent 1 BTC, due to a double-paste error. $ bitcoin-cli sendtoaddress 1address  1address
   1 <freik> It parses the 1 from the 2nd  '1address' and sent it
   2 <freik> guess i should be glad the next digit was hex
   

2. btcdrak renamed this:
   RPC validation checks for sendaddress
   RPC validation checks for sendtoaddress
   on Jun 3, 2015
3. 
   kanzure commented at 0:12 am on June 3, 2015: contributor

   There could be a set of mutation tests for common typos likely to happen with bitcoin-cli. Some of these may not be RPC issues?
4. 
   laanwj commented at 6:51 am on June 3, 2015: member

   This appears to be a problem with bitcoin-cli, not the RPC server. e.g. AmountFromValue calls value.get_real() which will raise an exception if the value is not a number.

   But bitcoin-cli’s command line to JSON parsing (RPCConvertValues) probably has no proper error handling. If so, I’m surprised this only gets discovered now.

5. 
   laanwj commented at 7:12 am on June 3, 2015: member

   I reproduce this on master:

   0src/bitcoin-cli  -testnet sendtoaddress msj42CCGruhRsFrGATiUuh25dtxYtnpbTx 1.0sds
   

   Paradoxally, it looks like #6121 solves it (the new JSON parser is more thorough?):

   0src/bitcoin-cli  -testnet sendtoaddress msj42CCGruhRsFrGATiUuh25dtxYtnpbTx 1.0sds
   1error: {"code":-32700,"message":"Parse error"}
   

6. 
   laanwj commented at 7:31 am on June 3, 2015: member

   I’ve narrowed it down to the JSON parser. Added the following tests, which should pass, but fail:

   0BOOST_CHECK_EQUAL(read_string(std::string("1.0sds"), value), false);
   1BOOST_CHECK_EQUAL(read_string(std::string("1.0]"), value), false);
   

   Somehow trailing characters don’t affect it. Prefixing character does. This passes:

   0BOOST_CHECK_EQUAL(read_string(std::string("[1.0"), value), false);
   

7. 
   jonasschnelli commented at 7:34 am on June 3, 2015: contributor

   UniValue does parse JSON more strict and handles some special cases manually. Maybe this is fixed after merging #6121 but it would need at least a fix for 0.11 if we like to get this fixed there. Will also investigate soon.
8. 
   laanwj commented at 7:39 am on June 3, 2015: member

   @jonasschnelli I’m already looking into this - read_string currently doesn’t check that it consumed the entire input stream, it’s an easy fix, which makes sense to merge into 0.10 and 0.11.
9. laanwj referenced this in commit a4993f7515 on Jun 3, 2015
10. 
    laanwj commented at 7:48 am on June 3, 2015: member

    See #6226
11. laanwj referenced this in commit 46f92fc9d8 on Jun 3, 2015
12. laanwj added the label Bug on Jun 3, 2015
13. laanwj referenced this in commit 4e157fc60d on Jun 3, 2015
14. laanwj closed this on Jun 3, 2015

    ---

15. laanwj referenced this in commit a7dcb7eb04 on Jun 3, 2015
16. laanwj referenced this in commit 5901596548 on Jun 3, 2015
17. laanwj referenced this in commit 181771b712 on Jun 3, 2015
18. reddink referenced this in commit 9bfca8c140 on May 27, 2020
19. DrahtBot locked this on Sep 8, 2021

Contributors
btcdrak kanzure laanwj jonasschnelli

Labels
Bug