Skip to content

Fix decoder bug when ending before decoding prefix#5455

Merged
apoelstra merged 1 commit intorust-bitcoin:masterfrom
shinghim:decoder-bug
Jan 2, 2026
Merged

Fix decoder bug when ending before decoding prefix#5455
apoelstra merged 1 commit intorust-bitcoin:masterfrom
shinghim:decoder-bug

Conversation

@shinghim
Copy link
Copy Markdown
Contributor

@shinghim shinghim commented Dec 26, 2025

Before this fix, calling ByteVecDecoder.end() on a decoder that hadn't finished reading in the full prefix would result in a valid result of an empty vec. This should instead result in an error, since the decoder shouldn't be able to decode something with an incomplete prefix.

Found from #5315 when i was testing some of the fuzzing:

INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 4008033836
INFO: Loaded 1 modules   (726 inline 8-bit counters): 726 [0x100aca050, 0x100aca326),
INFO: Loaded 1 PC tables (726 PCs): 726 [0x100aca328,0x100acd088),
INFO:       10 files found in /Users/shingng/git/rust-bitcoin/fuzz/corpus/consensus_encoding_decode_byte_vec
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes

thread '<unnamed>' (5327178) panicked at fuzz/fuzz_targets/consensus_encoding/decode_byte_vec.rs:21:17:
decoder should error when insufficient data provided
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
==12615== ERROR: libFuzzer: deadly signal
    #0 0x0001013453c4 in __sanitizer_print_stack_trace+0x28 (librustc-nightly_rt.asan.dylib:arm64+0x5d3c4)
    #1 0x000100a518d4 in fuzzer::PrintStackTrace()+0x30 (consensus_encoding_decode_byte_vec:arm64+0x1000158d4)
    #2 0x000100a45db8 in fuzzer::Fuzzer::CrashCallback()+0x54 (consensus_encoding_decode_byte_vec:arm64+0x100009db8)
    #3 0x00019914f740 in _sigtramp+0x34 (libsystem_platform.dylib:arm64+0x3740)
    #4 0x000199145884 in pthread_kill+0x124 (libsystem_pthread.dylib:arm64+0x6884)
    #5 0x00019904a84c in abort+0x78 (libsystem_c.dylib:arm64+0x7984c)
    #6 0x000100aa9e44 in _RNvNtNtNtCsk9AQ7OSayGk_3std3sys3pal4unix14abort_internal+0x8 (consensus_encoding_decode_byte_vec:arm64+0x10006de44)
    #7 0x000100aa9cd0 in _RNvNtCsk9AQ7OSayGk_3std7process5abort+0x8 (consensus_encoding_decode_byte_vec:arm64+0x10006dcd0)
    #8 0x000100aa5528 in _RNCNvCsaBYAWE6hvc2_13libfuzzer_sys10initialize0B3_+0xb8 (consensus_encoding_decode_byte_vec:arm64+0x100069528)
    #9 0x000100a9106c in _RNvNtCsk9AQ7OSayGk_3std9panicking15panic_with_hook+0x264 (consensus_encoding_decode_byte_vec:arm64+0x10005506c)
    #10 0x000100a85148 in _RNCNvNtCsk9AQ7OSayGk_3std9panicking13panic_handler0B5_+0x6c (consensus_encoding_decode_byte_vec:arm64+0x100049148)
    #11 0x000100a7cad4 in _RINvNtNtCsk9AQ7OSayGk_3std3sys9backtrace26___rust_end_short_backtraceNCNvNtB6_9panicking13panic_handler0zEB6_+0x8 (consensus_encoding_decode_byte_vec:arm64+0x100040ad4)
    #12 0x000100a8572c in _RNvCseYE12Li5r0M_7___rustc17rust_begin_unwind+0x1c (consensus_encoding_decode_byte_vec:arm64+0x10004972c)
    #13 0x000100aaa484 in _RNvNtCsh0x4TIixgmZ_4core9panicking9panic_fmt+0x24 (consensus_encoding_decode_byte_vec:arm64+0x10006e484)
    #14 0x000100a3d6ac in _RNvNvCsdWVpjOStM1p_34consensus_encoding_decode_byte_vec1__19___libfuzzer_sys_run decode_byte_vec.rs:45
    #15 0x000100a3f52c in rust_fuzzer_test_input lib.rs:276
    #16 0x000100a4436c in _RINvNvNtCsk9AQ7OSayGk_3std9panicking12catch_unwind7do_callNCNvCsaBYAWE6hvc2_13libfuzzer_sys15test_input_wrap0lEBY_+0xc4 (consensus_encoding_decode_byte_vec:arm64+0x10000836c)
    #17 0x000100a45034 in __rust_try+0x18 (consensus_encoding_decode_byte_vec:arm64+0x100009034)
    #18 0x000100a43c6c in LLVMFuzzerTestOneInput+0x16c (consensus_encoding_decode_byte_vec:arm64+0x100007c6c)
    #19 0x000100a47670 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)+0x158 (consensus_encoding_decode_byte_vec:arm64+0x10000b670)
    #20 0x000100a488e8 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<fuzzer::SizedFile, std::__1::allocator<fuzzer::SizedFile>>&)+0x240 (consensus_encoding_decode_byte_vec:arm64+0x10000c8e8)
    #21 0x000100a49058 in fuzzer::Fuzzer::Loop(std::__1::vector<fuzzer::SizedFile, std::__1::allocator<fuzzer::SizedFile>>&)+0x88 (consensus_encoding_decode_byte_vec:arm64+0x10000d058)
    #22 0x000100a676b8 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))+0x1aa4 (consensus_encoding_decode_byte_vec:arm64+0x10002b6b8)
    #23 0x000100a74158 in main+0x24 (consensus_encoding_decode_byte_vec:arm64+0x100038158)
    #24 0x000198d7dd50  (<unknown module>)

NOTE: libFuzzer has rudimentary signal handlers.
      Combine libFuzzer with AddressSanitizer or similar for better crash reports.
SUMMARY: libFuzzer: deadly signal
MS: 0 ; base unit: 0000000000000000000000000000000000000000


artifact_prefix='/Users/shingng/git/rust-bitcoin/fuzz/artifacts/consensus_encoding_decode_byte_vec/'; Test unit written to /Users/shingng/git/rust-bitcoin/fuzz/artifacts/consensus_encoding_decode_byte_vec/crash-da39a3ee5e6b4b0d3255bfef95601890afd80709
Base64:

────────────────────────────────────────────────────────────────────────────────

Failing input:

	fuzz/artifacts/consensus_encoding_decode_byte_vec/crash-da39a3ee5e6b4b0d3255bfef95601890afd80709

Output of `std::fmt::Debug`:

	[]

Reproduce with:

	cargo fuzz run consensus_encoding_decode_byte_vec fuzz/artifacts/consensus_encoding_decode_byte_vec/crash-da39a3ee5e6b4b0d3255bfef95601890afd80709

Minimize test case with:

	cargo fuzz tmin consensus_encoding_decode_byte_vec fuzz/artifacts/consensus_encoding_decode_byte_vec/crash-da39a3ee5e6b4b0d3255bfef95601890afd80709

────────────────────────────────────────────────────────────────────────────────

Error: Fuzz target exited with exit status: 77

@github-actions github-actions bot added the C-consensus_encoding PRs modifying the consensus-encoding crate label Dec 26, 2025
@shinghim
Fix decoder bug when ending before decoding prefix
5d4f9cf 
Before this fix, calling `ByteVecDecoder.end()` on a decoder that hadn't
finished reading in the full prefix would result in a valid result of an
empty vec. This should instead result in an error, since the decoder
shouldn't be able to decode something with an incomplete prefix.
@shinghim shinghim marked this pull request as ready for review December 26, 2025 17:20
@apoelstra
Copy link
Copy Markdown
Member

apoelstra commented Dec 27, 2025

Nice. I daresay the tests are a little too thorough (do we really need separate unit tests for 0xfd, 0xfe and 0xff?). But happy to ACK this anyway.

apoelstra
apoelstra approved these changes Dec 27, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 5d4f9cf; successfully ran local tests

tcharding
tcharding approved these changes Dec 30, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 5d4f9cf

@apoelstra apoelstra merged commit 5283f29 into rust-bitcoin:master Jan 2, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apoelstra apoelstra apoelstra approved these changes

@tcharding tcharding tcharding approved these changes

Assignees

No one assigned

Labels

C-consensus_encoding PRs modifying the consensus-encoding crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shinghim @apoelstra @tcharding