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

pull pierreN wants to merge 1 commits into bitcoin:master from pierreN:test-abs-ub changing 1 files +4 −0

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 :
```
$ echo -n "-9223372036854775808" > crash-abs-value
$ ./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).
pierreN cross-referenced this on Apr 1, 2020 from issue script: prevent UB when computing abs value for num opcode serialize by pierreN
fanquake added the label Tests on Apr 1, 2020
pierreN force-pushed on Apr 1, 2020
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? :)
tests: add CScriptNum::serialize to integer fuzz harness 7a21407741
pierreN force-pushed on Apr 1, 2020
pierreN renamed this:
~~tests: add fuzing harness for custom abs value functions~~
tests: add CScriptNum::serialize to integer fuzz harness
on Apr 1, 2020
pierreN commented at 4:40 PM on April 1, 2020: contributor

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

ACK 7a214077419b2a10568e495ca06eae1a42ffc7db
pierreN commented at 3:57 AM on April 2, 2020: contributor

Thanks. Note that tests fail since #18413 is not merged AFAIK.
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?
pierreN closed this on Apr 2, 2020
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
bitcoin locked this on Feb 15, 2022

Contributors

Labels

Linked (view graph)

#18046 Some fuzzing crashes when running under ASan and/or UBSan #18413 script: prevent UB when computing abs value for num opcode serialize