Skip to content

rpc: Fix implicit-integer-sign-change in gettxout #25095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 16, 2022
Merged

rpc: Fix implicit-integer-sign-change in gettxout #25095

merged 1 commit into from
May 16, 2022

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented May 9, 2022

No description provided.

@maflcko
Copy link
Member Author

maflcko commented May 9, 2022

Steps to reproduce after compiling with ubsan and loading the suppressions.

export UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1"

Reproduce with fuzzing:

$ echo 'Z2V0dHhvdXRclTuj7f07o239ensSAAAAAADPXv///////////////wAAAC4AAAAAAAAA//////9/BQWdbtUv/wJiZQ==' | base64 --decode > /tmp/crash_25095
$ FUZZ=rpc ./src/test/fuzz/fuzz /tmp/crash_25095
INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1406286977
INFO: Loaded 1 modules   (310988 inline 8-bit counters): 310988 [0x55d87dcdbe20, 0x55d87dd27cec), 
INFO: Loaded 1 PC tables (310988 PCs): 310988 [0x55d87dd27cf0,0x55d87e1e69b0), 
/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each.
Running: /root/fuzz_dir/scratch/fuzz_gen/code/crash-99578ec2fe87fa3602f8e029e32ef6a2016aed55
rpc/blockchain.cpp:997:25: runtime error: implicit conversion from type 'int' of value -65536 (32-bit, signed) to type 'uint32_t' (aka 'unsigned int') changed the value to 4294901760 (32-bit, unsigned)
    #0 0x55d87be6cc05 in gettxout()::$_15::operator()(RPCHelpMan const&, JSONRPCRequest const&) const src/./src/rpc/blockchain.cpp:997:25
    #1 0x55d87be6cc05 in std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), gettxout()::$_15>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9
    #2 0x55d87c744eac in std::function<UniValue (RPCHelpMan const&, JSONRPCRequest const&)>::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
    #3 0x55d87c742cb3 in RPCHelpMan::HandleRequest(JSONRPCRequest const&) const src/./src/rpc/util.cpp:583:26
    #4 0x55d87be4bb97 in CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const src/./rpc/server.h:109:91
    #5 0x55d87be4b7e2 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9
    #6 0x55d87bcb46b4 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
    #7 0x55d87bfb5077 in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) src/./src/rpc/server.cpp:474:20
    #8 0x55d87bfafb25 in ExecuteCommands(std::vector<CRPCCommand const*, std::allocator<CRPCCommand const*> > const&, JSONRPCRequest const&, UniValue&) src/./src/rpc/server.cpp:438:13
    #9 0x55d87bfaf6b2 in CRPCTable::execute(JSONRPCRequest const&) const src/./src/rpc/server.cpp:458:13
    #10 0x55d87b827478 in (anonymous namespace)::RPCFuzzTestingSetup::CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) src/./src/test/fuzz/rpc.cpp:54:18
    #11 0x55d87b827478 in rpc_fuzz_target(Span<unsigned char const>) src/./src/test/fuzz/rpc.cpp:361:28
    #12 0x55d87b5a1e42 in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2
    #13 0x55d87b8f511a in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
    #14 0x55d87b8f4d95 in LLVMFuzzerTestOneInput src/./src/test/fuzz/fuzz.cpp:154:5
    #15 0x55d87b4c5e32 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x13d3e32) (BuildId: aa1a8d81cff36c20e60c2ce5bf77057f6b27c54c)
    #16 0x55d87b4b03df in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x13be3df) (BuildId: aa1a8d81cff36c20e60c2ce5bf77057f6b27c54c)
    #17 0x55d87b4b60a7 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x13c40a7) (BuildId: aa1a8d81cff36c20e60c2ce5bf77057f6b27c54c)
    #18 0x55d87b4ded62 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x13ecd62) (BuildId: aa1a8d81cff36c20e60c2ce5bf77057f6b27c54c)
    #19 0x7fa1828480b2 in __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:308:16
    #20 0x55d87b4aad1d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x13b8d1d) (BuildId: aa1a8d81cff36c20e60c2ce5bf77057f6b27c54c)

SUMMARY: UndefinedBehaviorSanitizer: implicit-integer-sign-change rpc/blockchain.cpp:997:25 in

@maflcko maflcko marked this pull request as draft May 9, 2022 15:31
@DrahtBot
Copy link
Contributor

DrahtBot commented May 10, 2022 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

@DrahtBot DrahtBot mentioned this pull request May 11, 2022
Merged
@maflcko maflcko marked this pull request as ready for review May 13, 2022 09:39
@maflcko maflcko force-pushed the 2205-rpc-int-ubsan- branch from f80c82c to fa347a9 Compare May 13, 2022 09:40
theStack
theStack approved these changes May 16, 2022
View reviewed changes
Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-review ACK fa347a9

(didn't review the recent introduction of getInt to univalue in detail)

master:

$ ./src/bitcoin-cli gettxout ad67150fdb94478b431a0bb2c5cdf3a4e249d1f1a50fdfb9aad415dfb869c4b1 -2000000000
$ echo $?
0

PR:

$ ./src/bitcoin-cli gettxout ad67150fdb94478b431a0bb2c5cdf3a4e249d1f1a50fdfb9aad415dfb869c4b1 -2000000000
error code: -1
error message:
JSON integer out of range
$ echo $?
1

@fanquake fanquake merged commit b019cdc into bitcoin:master May 16, 2022
@maflcko maflcko deleted the 2205-rpc-int-ubsan-😛 branch May 16, 2022 13:27
@maflcko maflcko mentioned this pull request May 18, 2022
Merged
@luke-jr luke-jr mentioned this pull request May 20, 2022
Closed
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
@fanquake
Merge bitcoin#25095: rpc: Fix implicit-integer-sign-change in gettxout
503c0fc 
fa347a9 rpc: Fix implicit-integer-sign-change in gettxout (MacroFake)

Pull request description:

ACKs for top commit:
  theStack:
    Code-review ACK fa347a9

Tree-SHA512: 2a1128f714119b6b6cfeb20ee59c4f46404d5a83942253c73de64b0289a7d41e4437cf77d19b1cf623e2dd15fbaa1ec7acd03cc5d6dde41b3c7d06a082073ea1
@bitcoin bitcoin locked and limited conversation to collaborators May 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@theStack theStack theStack approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
RPC/REST/ZMQ
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maflcko @DrahtBot @theStack @fanquake