UBSan warning when fuzzing with -fsanitize=integer

Crypt-iQ commented at 7:20 am on August 7, 2020: contributor

When fuzzing the process_messages harness with -fsanitize=integer and taking care to use the ubsan suppressions file, I get the following crasher:

 0INFO: Seed: 2176449341
 1INFO: Loaded 1 modules   (204269 inline 8-bit counters): 204269 [0x555d1700aeb0, 0x555d1703cc9d), 
 2INFO: Loaded 1 PC tables (204269 PCs): 204269 [0x555d1703cca0,0x555d1735ab70), 
 3INFO:     7752 files found in /root/qa-assets/fuzz_seed_corpus/process_messages
 4INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 1048576 bytes
 5INFO: seed corpus: files: 7752 min: 1b max: 3984182b total: 71282676b rss: 81Mb
 6protocol.h:420:42: runtime error: implicit conversion from type 'int' of value -168430091 (32-bit, signed) to type 'unsigned int' changed the value to 4126537205 (32-bit, unsigned)
 7    [#0](/bitcoin-bitcoin/0/) 0x555d1630b5ff in CInv::IsGenTxMsg() const /root/bitcoin/src/./protocol.h:420:42
 8    [#1](/bitcoin-bitcoin/1/) 0x555d1630b5ff in ProcessMessage(CNode&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, CDataStream&, std::chrono::duration<long, std::ratio<1l, 1000000l> >, CChainParams const&, ChainstateManager&, CTxMemPool&, CConnman&, BanMan*, std::atomic<bool> const&) /root/bitcoin/src/net_processing.cpp:3696:25
 9    [#2](/bitcoin-bitcoin/2/) 0x555d1632d971 in PeerLogicValidation::ProcessMessages(CNode*, std::atomic<bool>&) /root/bitcoin/src/net_processing.cpp:3842:9
10    [#3](/bitcoin-bitcoin/3/) 0x555d1632fec5 in non-virtual thunk to PeerLogicValidation::ProcessMessages(CNode*, std::atomic<bool>&) /root/bitcoin/src/net_processing.cpp
11    [#4](/bitcoin-bitcoin/4/) 0x555d162a1f3e in ConnmanTestMsg::ProcessMessagesOnce(CNode&) /root/bitcoin/src/./test/util/net.h:26:56
12    [#5](/bitcoin-bitcoin/5/) 0x555d162a1f3e in test_one_input(std::vector<unsigned char, std::allocator<unsigned char> > const&) /root/bitcoin/src/test/fuzz/process_messages.cpp:74:21
13    [#6](/bitcoin-bitcoin/6/) 0x555d16954bd6 in LLVMFuzzerTestOneInput /root/bitcoin/src/test/fuzz/fuzz.cpp:45:5
14    [#7](/bitcoin-bitcoin/7/) 0x555d16231751 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/root/bitcoin/src/test/fuzz/process_messages+0x649751)
15    [#8](/bitcoin-bitcoin/8/) 0x555d16230e95 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) (/root/bitcoin/src/test/fuzz/process_messages+0x648e95)
16    [#9](/bitcoin-bitcoin/9/) 0x555d162337b7 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/root/bitcoin/src/test/fuzz/process_messages+0x64b7b7)
17    [#10](/bitcoin-bitcoin/10/) 0x555d16233b19 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) (/root/bitcoin/src/test/fuzz/process_messages+0x64bb19)
18    [#11](/bitcoin-bitcoin/11/) 0x555d162227ee in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/root/bitcoin/src/test/fuzz/process_messages+0x63a7ee)
19    [#12](/bitcoin-bitcoin/12/) 0x555d1624b632 in main (/root/bitcoin/src/test/fuzz/process_messages+0x663632)
20    [#13](/bitcoin-bitcoin/13/) 0x7f247597fb96 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b96)
21    [#14](/bitcoin-bitcoin/14/) 0x555d161f7569 in _start (/root/bitcoin/src/test/fuzz/process_messages+0x60f569)
22
23SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change protocol.h:420:42 in 
24MS: 0 ; base unit: 0000000000000000000000000000000000000000
250x6e,0x6f,0x74,0x66,0x6f,0x75,0x6e,0x64,0x0,0x1,0x7f,0x99,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xa5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0xf5,0x99,0x99,0x99,0x99,0x99,0x99,0x99,0x99,0x99,0x99,0x99,0x99,0xdc,0x1,0x7c,0x0,0x0,0x0,0x0,0x0,0x0,0xc0,
26notfound\x00\x01\x7f\x99\x02\x00\x00\x00\x00\x00\x00\x00\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xa5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\xf5\x99\x99\x99\x99\x99\x99\x99\x99\x99\x99\x99\x99\xdc\x01|\x00\x00\x00\x00\x00\x00\xc0
27artifact_prefix='./'; Test unit written to ./crash-6d18289c14a3cab8896d942d6c4021b2b6895a1e
28Base64: bm90Zm91bmQAAX+ZAgAAAAAAAAClpaWlpaWlpaWlpaWlpaWlpaX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX1mZmZmZmZmZmZmZmZ3AF8AAAAAAAAwA==

The problem occurs because type is an int and gets converted in the comparison. I feel like type should be uint32_t but felt I’d make an issue to see if this should be a suppression instead :). https://github.com/bitcoin/bitcoin/blob/a78742830aa35bf57bcb0a4730977a1e5a1876bc/src/protocol.h#L420

Crypt-iQ added the label Bug on Aug 7, 2020

Crypt-iQ commented at 2:27 pm on August 8, 2020: contributor

This happens here: https://github.com/bitcoin/bitcoin/blob/6d8543504d8c5bde1d12a3c60407dee44d2c8e11/src/net_processing.cpp#L3719-L3727

vInv is populated with a CInv with negative type and then IsGenTxMsg is called slightly lower. Just for more preciseness so folks don’t have to go searching.

jonatack commented at 4:19 pm on August 25, 2020: member

Nice find!

jonatack referenced this in commit 533a9a0d2c on Aug 25, 2020

jonatack referenced this in commit 125468af9c on Aug 26, 2020

jonatack referenced this in commit 407175e0c2 on Aug 27, 2020

practicalswift commented at 6:22 pm on August 27, 2020: contributor

Thanks for fuzzing and reporting your findings @Crypt-iQ!

laanwj added the label Tests on Sep 3, 2020

laanwj removed the label Bug on Sep 3, 2020

laanwj closed this on Sep 3, 2020

sidhujag referenced this in commit addddc0053 on Sep 3, 2020

DrahtBot locked this on Feb 15, 2022

UBSan warning when fuzzing with -fsanitize=integer #19678