rpc: Quote user supplied strings in error messages #23755

1. 
   MarcoFalke commented at 8:49 am on December 13, 2021: member

   I can’t see a downside doing this and this fixes a fuzzing crash

   Background:

   This is a follow-up to commit 926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090, which introduced the “starts_with-hack”. Maybe an alternative to the hack would be to assign a unique error code to internal bugs? However, I think this can be done in an separate pull request and the changes here make sense even on their own.

2. MarcoFalke added the label RPC/REST/ZMQ on Dec 13, 2021
3. Dunlap28 approved
4. 
   MarcoFalke commented at 9:50 am on December 13, 2021: member

   For reference, the fuzzing crash is:

   0# FUZZ=rpc /root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz ./scratch/fuzz_gen/code/crash-ae08cae614be78d63c13243bbc07c490782edad0
   1INFO: Running with entropic power schedule (0xFF, 100).
   2INFO: Seed: 3547005882
   3INFO: Loaded 1 modules   (306062 inline 8-bit counters): 306062 [0x55cd7c2bd780, 0x55cd7c30830e),
   4INFO: Loaded 1 PC tables (306062 PCs): 306062 [0x55cd7c308310,0x55cd7c7b3bf0),
   5/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each.
   6Running: ./scratch/fuzz_gen/code/crash-ae08cae614be78d63c13243bbc07c490782edad0
   7fuzz: test/fuzz/rpc.cpp:367: void rpc_fuzz_target(FuzzBufferType): Assertion `error_msg.find("trigger_internal_bug") != std::string::npos' failed.
   

   0# xxd ./scratch/fuzz_gen/code/crash-ae08cae614be78d63c13243bbc07c490782edad0
   100000000: 6765 7474 786f 7574 7365 7469 6e66 6f5c  gettxoutsetinfo\
   200000010: 0149 6e74 6572 6e61 6c20 6275 6720 6465  .Internal bug de
   300000020: 7465 6374 6564 6f64 7979 7a5a 5a5a 6f64  tectedodyyzZZZod
   400000030: 7979 7aff 7987 8d                        yyz.y..
   

   0# base64 ./scratch/fuzz_gen/code/crash-ae08cae614be78d63c13243bbc07c490782edad0
   1Z2V0dHhvdXRzZXRpbmZvXAFJbnRlcm5hbCBidWcgZGV0ZWN0ZWRvZHl5elpaWm9keXl6/3mHjQ==
   

5. fanquake approved
6. 
   fanquake commented at 2:02 pm on December 13, 2021: member

   ACK fa416f5f4c155664c801cf5bd577d2e233942278 - depending on how far you want to take this, there are certainly more strings that could be quoted:

   https://github.com/bitcoin/bitcoin/blob/bf66e258a84e18935fde3ebb9a4b0392bf883222/src/rpc/blockchain.cpp#L2216

   0src/bitcoin-cli getblockstats 590000 '["blah"]'                           
   1error code: -8
   2error message:
   3Invalid selected statistic blah
   

7. rpc: Quote user supplied strings in error messages fa24a3df87
8. MarcoFalke force-pushed on Dec 13, 2021
9. 
   MarcoFalke commented at 2:20 pm on December 13, 2021: member

   ACK fa416f5 - depending on how far you want to take this, there are certainly more strings that could be quoted:

   Thanks, done.

10. fanquake approved
11. 
    fanquake commented at 2:25 pm on December 13, 2021: member

    ACK fa24a3df8796cbf4eeb35d950a4c848d605e5b22 - to fix the fuzzers.
12. MarcoFalke merged this on Dec 13, 2021
13. MarcoFalke closed this on Dec 13, 2021

    ---

14. MarcoFalke deleted the branch on Dec 13, 2021
15. 
    jamesob commented at 4:39 pm on December 13, 2021: member

    Posthumous ACK
16. sidhujag referenced this in commit 81a7d013ab on Dec 13, 2021
17. MarcoFalke referenced this in commit a1e04e1079 on Dec 31, 2021
18. sidhujag referenced this in commit 43040aabbb on Dec 31, 2021
19. DrahtBot locked this on Dec 13, 2022

Contributors
MarcoFalke Dunlap28 fanquake jamesob

Labels
RPC/REST/ZMQ