tests: add CScriptNum::serialize to integer fuzz harness #18491

1. 
   pierreN commented at 9:01 am on April 1, 2020: contributor

   Add a fuzzing harness for CScriptNum::serialize, core_write.cpp:ValueFromAmount and util/moneystr.cpp:FormatMoney.

   Those functions manually compute absolute values of int64_t numbers which can lead to undefined behavior, see : #18413 #18046

   You can trigger this new harness with the following input :

   0$ echo -n "-9223372036854775808" > crash-abs-value
   1$ ./src/test/fuzz/abs_ub crash-abs-value
   

   Note that BitcoinUnits::format also does the same (but requires QT headers to compile so I’m not sure we can add it to the fuzzer).

2. fanquake added the label Tests on Apr 1, 2020
3. pierreN force-pushed on Apr 1, 2020
4. 
   practicalswift commented at 2:01 pm on April 1, 2020: contributor

   @pierreN Thanks for contributing to the fuzzers.

   ValueFromAmount is already fuzzed in src/test/fuzz/integer.cpp.

   FormatMoney is already fuzzed in src/test/fuzz/integer.cpp.

   That leaves CScriptNum::serialize(…) which isn’t fuzzed directly today AFAICT.

   Instead of adding a new fuzzing file just for CScriptNum::serialize(…) I suggest adding the line CScriptNum::serialize(i64) to the existing src/test/fuzz/integer.cpp. WDYT? :)

5. tests: add CScriptNum::serialize to integer fuzz harness 7a21407741
6. pierreN force-pushed on Apr 1, 2020
7. pierreN renamed this:
   tests: add fuzing harness for custom abs value functions
   tests: add CScriptNum::serialize to integer fuzz harness
   on Apr 1, 2020
8. 
   pierreN commented at 4:40 pm on April 1, 2020: contributor

   Ow, my bad. Thanks, I updated the PR accordingly.
9. 
   practicalswift commented at 6:25 pm on April 1, 2020: contributor

   ACK 7a214077419b2a10568e495ca06eae1a42ffc7db
10. 
    pierreN commented at 3:57 am on April 2, 2020: contributor

    Thanks. Note that tests fail since #18413 is not merged AFAIK.
11. 
    MarcoFalke commented at 1:02 pm on April 2, 2020: member

    Thanks. Note that tests fail since #18413 is not merged AFAIK.

    Could add the tests to that pull and close this one?

12. pierreN closed this on Apr 2, 2020

    ---

13. 
    pierreN commented at 1:20 pm on April 2, 2020: contributor

    Yes good idea, I just added the test to the original PR : https://github.com/bitcoin/bitcoin/commit/c6819c4fb756b6c8fa5ad5ab7b77081acb30feb0
14. DrahtBot locked this on Feb 15, 2022

Contributors
pierreN practicalswift MarcoFalke

Labels
Tests