psbt: Missing validation that deserialization consumes exact number of bytes specified in length prefix #2424
Description
By running differential fuzzing between Bitcoin Core and btcd, we got a crash where btcd successfully parsed a PSBT and Bitcoin Core returned an error due to Size of value was not the stated size. This error is returned by the following function where it checks if the unserialized content has exactly the specified size (e.g. <value> := <valuelen> <valuedata>):
template<typename Stream, typename... X>
void UnserializeFromVector(Stream& s, X&&... args)
{
size_t expected_size = ReadCompactSize(s);
size_t remaining_before = s.size();
UnserializeMany(s, args...);
size_t remaining_after = s.size();
if (remaining_after + expected_size != remaining_before) {
throw std::ios_base::failure("Size of value was not the stated size");
}
}I did a quick review on btcd's psbt code and noticed that it's not checking the data has the exact specified size. It just reads based on the size but doesn't validate if there is any remaining byte.
// Next we parse the GLOBAL section. There is currently only 1 known
// key type, UnsignedTx. We insist this exists first; unknowns are
// allowed, but only after.
keyCode, keyData, err := getKey(r)
if err != nil {
return nil, err
}
if GlobalType(keyCode) != UnsignedTxType || keyData != nil {
return nil, ErrInvalidPsbtFormat
}
// Now that we've verified the global type is present, we'll decode it
// into a proper unsigned transaction, and validate it.
value, err := wire.ReadVarBytes(
r, 0, MaxPsbtValueLength, "PSBT value",
)
if err != nil {
return nil, err
}
msgTx := wire.NewMsgTx(2)
// BIP-0174 states: "The transaction must be in the old serialization
// format (without witnesses)."
err = msgTx.DeserializeNoWitness(bytes.NewReader(value))
if err != nil {
return nil, err
}Perhaps a fix (haven't tested):
// format (without witnesses)."
- err = msgTx.DeserializeNoWitness(bytes.NewReader(value))
+ reader := bytes.NewReader(value)
+ err = msgTx.DeserializeNoWitness(reader)
if err != nil {
return nil, err
}
+
+ if reader.Len() != 0 {
+ return nil, ErrInvalidPsbtFormat
+ }
if !validateUnsignedTX(msgTx) {
return nil, ErrInvalidRawTxSigned
}