ECDSA pubkey recovery succeeds with invalid signature

[guidovranken]

#include <botan/ecdsa.h>

int main(void)
{
    const auto pub = ::Botan::ECDSA_PublicKey(
            ::Botan::EC_Group("secp256k1"),
            {0},
            ::Botan::BigInt("115792089237316195423570985008687907852837564279374904382605163141518161494307"),
            ::Botan::BigInt("115792089237316195423570985008687907852837564279074904382605163141518161494336"),
            1);

    printf("%s\n%s\n",
            pub.public_point().get_affine_x().to_dec_string().c_str(),
            pub.public_point().get_affine_y().to_dec_string().c_str());

    return 0;
}

In this PoC the signature S is larger than curve order. libsecp256k1 rejects this.

Is this intentionally allowed or a bug?

edit: Fixed link.

[randombit]

Not intentional - missing a check r/s are within range of the order. Tests here are currently insufficient - we don't have any negative tests at all.

[guidovranken]

I also found an input where S = 0 and Botan recovers a pubkey, meaning the lower bound isn't checked either.

Also I think the recovery ID shouldn't be allowed to be 4:

botan/src/lib/pubkey/ecdsa/ecdsa.cpp

Line 39 in 8ba41c3

if(v > 4)

For reference see libsecp256k1: https://github.com/bitcoin-core/secp256k1/blob/c083cc6e52a3ab749f5451de9c515d75897649c6/src/modules/recovery/main_impl.h#L46

[randombit]

Fixed on master now - checking 0 < r,s < order and v < 4