brunoerg / bitcoinfuzz

Differential Fuzzing of Bitcoin implementations and libraries
31 stars 11 forks source link

addrv2: rust-bitcoin returns invalid torv2 address #50

Open brunoerg opened 2 months ago

brunoerg commented 2 months ago

I just got a crash in addrv2 (https://github.com/brunoerg/bitcoinfuzz/pull/48) target because when deserializing addrv2 addresses rust-bitcoin checks whether TorV2 address is valid and throws an error if it is invalid. However, when deserializing an addrv2, Bitcoin Core doesn't validate TorV2 anymore (Core removed support for torv2 - https://github.com/bitcoin/bitcoin/pull/22050) and simply ignore it. So, the message is valid but the addr is ignored.

3 => {
    if len != 10 {
        return Err(encode::Error::ParseFailed("Invalid TorV2 address"));
    }
    let id = Decodable::consensus_decode(r)?;
    AddrV2::TorV2(id)
}

However, it might be an issue in Bitcoin Core, because even not supporting torv2, it should validate the length according to the BIP.

brunoerg commented 2 months ago

hmmm

https://github.com/bitcoin/bitcoin/pull/22050/files#r645432156

apoelstra commented 2 months ago

I'd consider this an upstream issue. Though for us, maybe we should drop TorV2 support.

brunoerg commented 2 months ago

Sounds good to drop TorV2 support, it's basically useless.