Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ECDSA public key recovery: Internal error: False assertion m_point is not null in EC_AffinePoint #4208

Closed
guidovranken opened this issue Jul 13, 2024 · 6 comments

Comments

@guidovranken
Copy link

#include <botan/system_rng.h>
#include <botan/ecdsa.h>

int main(void)
{
    std::unique_ptr<::Botan::EC_Group> group = std::make_unique<::Botan::EC_Group>("secp256k1");
    const std::vector<uint8_t> msg = {
        0x56, 0xD0, 0x03, 0x43, 0x31, 0x9D, 0xF5, 0xE2, 0x6B, 0xFC, 0x0D, 0x4C, 0xBE, 0xA8, 0x67, 0x90,
        0xD6, 0x62, 0xA9, 0x2C, 0xEE, 0x96, 0x60, 0xBD, 0x1C, 0x2F, 0xB7, 0x9E, 0xBF, 0x2F, 0xF1, 0x47};

    {
        const ::Botan::BigInt R("64018897");
        const ::Botan::BigInt S("392");

        std::unique_ptr<::Botan::ECDSA_PublicKey> pub = nullptr;
        try {
            pub = std::make_unique<::Botan::ECDSA_PublicKey>(*group, msg, R, S, 0);
        } catch ( ::Botan::Invalid_State& e ) {
        } catch ( ::Botan::Decoding_Error& ) {
        } catch ( ::Botan::Invalid_Argument& ) {
        }

    }

    return 0;
}

This always used to work but now throws:

terminate called after throwing an instance of 'Botan::Internal_Error'
  what():  Internal error: False assertion m_point is not null in EC_AffinePoint @src/lib/pubkey/ec_group/ec_apoint.cpp:16
Aborted

Confirming with you that I should catch Botan::Internal_Error at this point in my fuzzer?

@randombit
Copy link
Owner

Confirming with you that I should catch Botan::Internal_Error at this point in my fuzzer?

No, if we throw Internal_Error it means something went wrong that we did not expect, if it ever occurs it's a bug.

There is a fix for what you're seeing in #4203 but that's a larger PR. I'll split out the fix and merge asap to reduce noise on your end.

FYI I'm about halfway through a complete rewrite of the elliptic curve code (#4027) for the major curves (NIST, Brainpool, SM2, secp256k1) expect to see some funny things happen. Nothings going to break in the public API (well not on purpose anyway) but I know cryptofuzz accesses internals directly and there you may see breakage.

@guidovranken
Copy link
Author

There is a fix for what you're seeing in #4203 but that's a larger PR. I'll split out the fix and merge asap to reduce noise on your end.

Thanks, it can wait though because I'm temporarily catching the error anyway:

https://github.com/guidovranken/cryptofuzz/blob/7ce31e3ae42980ce7488ad98f51e9f8ea0b5b84c/modules/botan/module.cpp#L1310-L1313

I'll remove that catch once the bug is fixed.

@randombit
Copy link
Owner

Fixed on master

@guidovranken
Copy link
Author

Still get:

terminate called after throwing an instance of 'Botan::Invalid_Argument'
what(): Unexpected v param for ECDSA public key recovery

@randombit
Copy link
Owner

Testcase posted in first comment works for me with latest master (25356e1)

@guidovranken
Copy link
Author

Sorry, nevermind, I was confusing Botan::Internal_Error and Botan::Invalid_Argument

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants