Merge rust-bitcoin#5455: Fix decoder bug when ending before decoding prefix

5d4f9cf Fix decoder bug when ending before decoding prefix (Shing Him Ng)

Pull request description:

  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 rust-bitcoin#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)
      rust-bitcoin#18 0x000100a43c6c in LLVMFuzzerTestOneInput+0x16c (consensus_encoding_decode_byte_vec:arm64+0x100007c6c)
      rust-bitcoin#19 0x000100a47670 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long)+0x158 (consensus_encoding_decode_byte_vec:arm64+0x10000b670)
      rust-bitcoin#20 0x000100a488e8 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::__1::vector<fuzzer::SizedFile, std::__1::allocator<fuzzer::SizedFile>>&)+0x240 (consensus_encoding_decode_byte_vec:arm64+0x10000c8e8)
      rust-bitcoin#21 0x000100a49058 in fuzzer::Fuzzer::Loop(std::__1::vector<fuzzer::SizedFile, std::__1::allocator<fuzzer::SizedFile>>&)+0x88 (consensus_encoding_decode_byte_vec:arm64+0x10000d058)
      rust-bitcoin#22 0x000100a676b8 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long))+0x1aa4 (consensus_encoding_decode_byte_vec:arm64+0x10002b6b8)
      rust-bitcoin#23 0x000100a74158 in main+0x24 (consensus_encoding_decode_byte_vec:arm64+0x100038158)
      rust-bitcoin#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
  ```


ACKs for top commit:
  apoelstra:
    ACK 5d4f9cf; successfully ran local tests
  tcharding:
    ACK 5d4f9cf


Tree-SHA512: 89298839cfc33b2cf3379ca5ddcbf3edb94b81d5dfae01ca23b034e390f01653a1a147642928313ae02a2136a9a9397f6f66d369abb12786016ac2186638f076

Author: apoelstra

consensus_encoding/src/decode/decoders.rs

@@ -107,6 +107,12 @@ impl Decoder for ByteVecDecoder {
 
     fn end(self) -> Result<Self::Output, Self::Error> {
         use {ByteVecDecoderError as E, ByteVecDecoderErrorInner as Inner};
+        
+        if let Some(ref prefix_decoder) = self.prefix_decoder {  
+            return Err(E(Inner::UnexpectedEof(UnexpectedEofError {
+                missing: prefix_decoder.read_limit(), 
+            })));
+        }
 
         if self.bytes_written == self.bytes_expected {
             Ok(self.buffer)
@@ -1208,6 +1214,65 @@ mod tests {
             decode_byte_vec_multi_byte_length_prefix, [0xff; 256], two_fifty_six_bytes_encoded();
     }
 
+    #[test]
+    #[cfg(feature = "alloc")]
+    fn byte_vec_decoder_decode_empty_slice() {
+        let mut decoder = ByteVecDecoder::new();
+        let data = [];
+        let _ = decoder.push_bytes(&mut data.as_slice());
+        let err = decoder.end().unwrap_err();
+        
+        if let ByteVecDecoderErrorInner::UnexpectedEof(e) = err.0 {
+            assert_eq!(e.missing, 1);
+        } else {
+            panic!("Expected UnexpectedEof error");
+        }
+    }
+
+    #[test]
+    #[cfg(feature = "alloc")]
+    fn byte_vec_decoder_incomplete_0xfd_prefix() {
+        let mut decoder = ByteVecDecoder::new();
+        let data = [0xFD];
+        let _ = decoder.push_bytes(&mut data.as_slice());
+        let err = decoder.end().unwrap_err();
+
+        if let ByteVecDecoderErrorInner::UnexpectedEof(e) = err.0 {
+            assert_eq!(e.missing, 2);
+        } else {
+            panic!("Expected UnexpectedEof error");
+        }
+    }
+
+    #[test]
+    #[cfg(feature = "alloc")]
+    fn byte_vec_decoder_incomplete_0xfe_prefix() {
+        let mut decoder = ByteVecDecoder::new();
+        let data = [0xFE];
+        let _ = decoder.push_bytes(&mut data.as_slice());
+        let err = decoder.end().unwrap_err();
+        
+        if let ByteVecDecoderErrorInner::UnexpectedEof(e) = err.0 {
+            assert_eq!(e.missing, 4);
+        } else {
+            panic!("Expected UnexpectedEof error");
+        }
+    }
+
+    #[test]
+    #[cfg(feature = "alloc")]
+    fn byte_vec_decoder_incomplete_0xff_prefix() {
+        let mut decoder = ByteVecDecoder::new();
+        let data = [0xFF];
+        let _ = decoder.push_bytes(&mut data.as_slice());
+        let err = decoder.end().unwrap_err();
+        
+        if let ByteVecDecoderErrorInner::UnexpectedEof(e) = err.0 {
+            assert_eq!(e.missing, 8);
+        } else {
+            panic!("Expected UnexpectedEof error");
+        }
+    }
     #[test]
     #[cfg(feature = "alloc")]
     fn byte_vec_decoder_reserves_in_batches() {