search

brunoerg / bitcoinfuzz

Differential Fuzzing of Bitcoin implementations and libraries
32 stars 12 forks source link

psbt: crash on checking key type #44

Closed brunoerg closed 3 weeks ago

brunoerg commented 5 months ago

We just got a crash on psbt target. rust-miniscript successfully deserializes a PSBT while Bitcoin Core fails due to ReadCompactSize(): size too large. This failure happens during key type checking.

// Type is compact size uint at beginning of key
SpanReader skey{key};
uint64_t type = ReadCompactSize(skey);

Base64: cHNidP8BABwAAAAFAAL9///+/wAAAAAALwBzYnT/AQAAAQEAdP90/wEAHAAAAAUAAv3///4B///Q////xgBzYnT/AQEAAC0AHAAx/3V0LakrcP8BdP8BAAAB/wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD3/Mf91L62pK6k=

hax0kartik commented 5 months ago

Hello, I tried to find the root cause of this bug and there seem to be atleast 2 issues present:

  1. Rust-Bitcoin seems to assume that type_value is u8 which is wrong. According to the documentation, it should be a VarInt(check keytype documentation in specification)
  2. This check seems to be missing.
brunoerg commented 5 months ago

cc: @apoelstra

apoelstra commented 5 months ago

Thanks for finding this! I have subscribed to the repo so you hopefully won't need to ping me in future.

This looks reasonably straightforward to fix although it will involve prodding the psbt module which is in a state of limbo as we explore rewriting it.

brunoerg commented 5 months ago

Thank you, @apoelstra.

brunoerg commented 3 weeks ago

Closed as resolved. All impl has fixed it.