Fix buffer over-read

[nikhil-gupta-tw]

This pull request strengthens the validation of ECDSA signature operations by enforcing a minimum digest size and updating the relevant signing and verification logic. It also adds new tests to ensure that short digests are properly rejected. These changes help prevent potential misuse of cryptographic functions with invalid input lengths.

Validation improvements for ECDSA operations:

• Introduced a constant kEcdsaMinDigestSize (32 bytes) in src/PublicKey.cpp to define the minimum allowed digest size for ECDSA operations.
• Updated PublicKey::verify and PublicKey::verifyAsDER to explicitly reject digests shorter than the minimum size for SECP256k1 and NIST256p1 key types, returning false if the input is too short. [1] [2]
• Changed PrivateKey::signAsDER to use ecdsa_sign_digest_checked, which enforces digest size validation during signing.

Test coverage enhancements:

• Added SignAsDERRejectsShortDigest test to verify that signing with a short digest returns an empty result.
• Added VerifyRejectsShortDigest and VerifyAsDERRejectsShortDigest tests to confirm that verification functions reject signatures with short digests.

[Copilot]

Pull request overview

This PR hardens ECDSA signing and verification against short digests to prevent potential buffer over-reads, and adds regression tests to ensure short digests are rejected.

Changes:

• Add a minimum digest-length guard (32 bytes) for ECDSA verification in PublicKey::verify / verifyAsDER.
• Route DER signing through a digest-length-checked ECDSA signing helper.
• Add tests ensuring signing/verifying with short digests fails.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/PublicKey.cpp	Adds a 32-byte minimum digest check before calling ecdsa_verify_digest for ECDSA key types.
src/PrivateKey.cpp	Switches DER signing to a checked ECDSA signing function that rejects short digests.
tests/common/PublicKeyTests.cpp	Adds tests asserting ECDSA verify paths reject short digests.
tests/common/PrivateKeyTests.cpp	Adds a test asserting DER signing rejects short digests.

Comments suppressed due to low confidence (1)

src/PublicKey.cpp:224

• The PR description says verifyAsDER rejects short digests for both SECP256k1 and NIST256p1, but the implementation of PublicKey::verifyAsDER only handles TWPublicKeyTypeSECP256k1/SECP256k1Extended and returns false for all other key types. Either update the PR description to match the actual supported behavior, or extend verifyAsDER to support NIST256p1 if that’s intended.

bool PublicKey::verifyAsDER(const Data& signature, const Data& message) const {
    switch (type) {
    case TWPublicKeyTypeSECP256k1:
    case TWPublicKeyTypeSECP256k1Extended: {
        if (message.size() < kEcdsaMinDigestSize) { return false; }
        Data sig(64);
        int ret = ecdsa_sig_from_der(signature.data(), signature.size(), sig.data());
        if (ret) {
            return false;
        }
        return ecdsa_verify_digest(&secp256k1, bytes.data(), sig.data(), message.data()) == 0;
    }

    default:
        return false;
    }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Binary size comparison

➡️ aarch64-apple-ios: 14.34 MB

➡️ aarch64-apple-ios-sim: 14.34 MB

➡️ aarch64-linux-android: 18.77 MB

➡️ armv7-linux-androideabi: 16.20 MB

➡️ wasm32-unknown-emscripten: 13.68 MB