fix(public-key): enforce message length validation for ECDSA signatures

src/PrivateKey.cpp

@@ -256,10 +256,9 @@ PublicKey PrivateKey::getPublicKey(TWPublicKeyType type) const {
 }
 
 int ecdsa_sign_digest_checked(const ecdsa_curve* curve, const uint8_t* priv_key, const uint8_t* digest, size_t digest_size, uint8_t* sig, uint8_t* pby, int (*is_canonical)(uint8_t by, uint8_t sig[64])) {
-    if (digest_size < 32) {
+    if (digest_size != PrivateKey::ecdsaMessageSize) {
         return -1;
     }
-    assert(digest_size >= 32);
     return ecdsa_sign_digest(curve, priv_key, digest, sig, pby, is_canonical);
 }
 
@@ -371,7 +370,7 @@ Data PrivateKey::signAsDER(const Data& digest) const {
     }
     Data sig(64);
     bool success =
-        ecdsa_sign_digest(&secp256k1, key().data(), digest.data(), sig.data(), nullptr, nullptr) == 0;
+        ecdsa_sign_digest_checked(&secp256k1, key().data(), digest.data(), digest.size(), sig.data(), nullptr, nullptr) == 0;
     if (!success) {
         return {};
     }

src/PrivateKey.h

@@ -20,6 +20,8 @@ class PrivateKey {
     static const size_t _size = 32;
     /// The number of bytes in a Cardano key (two extended ed25519 keys + chain code)
     static const size_t cardanoKeySize = 2 * 3 * 32;
+    /// The number of bytes in an ECDSA message digest (e.g., 32-byte hash) that can be signed.
+    static const size_t ecdsaMessageSize = 32;
 
     /// The private key bytes:
     /// - common case: 'size' bytes

src/PublicKey.cpp

@@ -20,7 +20,7 @@
 
 namespace TW {
 
-bool validateSignatureLength(TWPublicKeyType type, const Data& signature) {
+static bool validateSignatureLength(TWPublicKeyType type, const Data& signature) {
     switch (type) {
     case TWPublicKeyTypeSECP256k1:
     case TWPublicKeyTypeSECP256k1Extended:
@@ -35,6 +35,27 @@ bool validateSignatureLength(TWPublicKeyType type, const Data& signature) {
     }
 }
 
+static bool validateMessageLength(TWPublicKeyType type, const Data& message) {
+    switch (type) {
+    case TWPublicKeyTypeED25519:
+    case TWPublicKeyTypeCURVE25519:
+    case TWPublicKeyTypeED25519Blake2b:
+    case TWPublicKeyTypeED25519Cardano:
+        // Allow any message size for ed25519.
+        return true;
+    case TWPublicKeyTypeSECP256k1:
+    case TWPublicKeyTypeNIST256p1:
+    case TWPublicKeyTypeSECP256k1Extended:
+    case TWPublicKeyTypeNIST256p1Extended:
+        return message.size() == PublicKey::ecdsaMessageSize;
+    case TWPublicKeyTypeStarkex:
+        // Digest shorter than 32 bytes will be left-padded with zeros before verification.
+        return message.size() <= PublicKey::starkexMessageMaxSize;
+    default:
+        return false;
+    }
+}
+
 /// Determines if a collection of bytes makes a valid public key of the
 /// given type.
 bool PublicKey::isValid(const Data& data, enum TWPublicKeyType type) {
@@ -165,6 +186,9 @@ bool PublicKey::verify(const Data& signature, const Data& message) const {
     if (!validateSignatureLength(type, signature)) {
         return false;
     }
+    if (!validateMessageLength(type, message)) {
+        return false;
+    }
 
     switch (type) {
     case TWPublicKeyTypeSECP256k1:
@@ -183,7 +207,7 @@ bool PublicKey::verify(const Data& signature, const Data& message) const {
     }
     case TWPublicKeyTypeCURVE25519: {
         auto ed25519PublicKey = Data();
-        ed25519PublicKey.resize(PublicKey::ed25519Size);
+        ed25519PublicKey.resize(ed25519Size);
         curve25519_pk_to_ed25519(ed25519PublicKey.data(), bytes.data());
 
         ed25519PublicKey[31] &= 0x7F;
@@ -203,6 +227,13 @@ bool PublicKey::verify(const Data& signature, const Data& message) const {
 }
 
 bool PublicKey::verifyAsDER(const Data& signature, const Data& message) const {
+    if (signature.size() < derSignatureMinSize || signature.size() > derSignatureMaxSize) {
+        return false;
+    }
+    if (message.size() != ecdsaMessageSize) {
+        return false;
+    }
+
     switch (type) {
     case TWPublicKeyTypeSECP256k1:
     case TWPublicKeyTypeSECP256k1Extended: {

src/PublicKey.h

@@ -37,9 +37,20 @@ class PublicKey {
     /// The number of bytes in a secp256k1 signature.
     static const size_t secp256k1SignatureSize = 65;
 
-    /// Magic number used in V compnent encoding
+    /// Magic number used in V component encoding
     static const byte SignatureVOffset = 27;
 
+    /// The number of bytes in an ECDSA message digest (e.g., 32-byte hash) that can be verified.
+    static const size_t ecdsaMessageSize = 32;
+    /// The maximum number of bytes in a message that can be verified with a starkex public key.
+    /// Digest shorter than 32 bytes will be left-padded with zeros before verification.
+    static const size_t starkexMessageMaxSize = 32;
+
+    /// The minimum number of bytes in a valid DER-encoded ECDSA signature.
+    static const size_t derSignatureMinSize = 2;
+    /// The maximum number of bytes in a valid DER-encoded ECDSA signature.
+    static const size_t derSignatureMaxSize = 72;
+
     /// The public key bytes.
     Data bytes;
 

tests/common/PrivateKeyTests.cpp

@@ -323,6 +323,24 @@ TEST(PrivateKey, SignShortDigest) {
     }
 }
 
+TEST(PrivateKey, SignLongDigest) {
+    Data privKeyData = parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5");
+    auto privateKey = PrivateKey(privKeyData);
+    Data shortDigest = TW::data("123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef012");
+    {
+        Data actual = privateKey.sign(shortDigest, TWCurveSECP256k1);
+        EXPECT_EQ(actual.size(), 0ul);
+    }
+    {
+        Data actual = privateKey.sign(shortDigest, TWCurveNIST256p1);
+        EXPECT_EQ(actual.size(), 0ul);
+    }
+    {
+        Data actual = privateKey.sign(shortDigest, TWCurveSECP256k1, isCanonical);
+        EXPECT_EQ(actual.size(), 0ul);
+    }
+}
+
 TEST(PrivateKey, SignWithDifferentCurveWorks) {
     Data privKeyData = parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5");
     // Using the deprecated constructor without specifying a curve
@@ -372,4 +390,10 @@ TEST(PrivateKey, SignWithDifferentCurveThrows) {
     }
 }
 
+TEST(PrivateKey, SignAsDERRejectsShortDigest) {
+    const auto privateKey = PrivateKey(parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5"));
+    const auto shortDigest = parse_hex("0102030405060708");
+    EXPECT_TRUE(privateKey.signAsDER(shortDigest).empty());
+}
+
 } // namespace TW::tests

tests/common/PublicKeyTests.cpp

@@ -366,3 +366,37 @@ TEST(PublicKeyTests, isValidED25519) {
     EXPECT_FALSE(PublicKey::isValid(parse_hex("0101beff0e5d6f6e6e6d573d3044f3e2bfb353400375dc281da3337468d4aa527908"), TWPublicKeyTypeED25519));
     EXPECT_FALSE(PublicKey(parse_hex("0399c6f51ad6f98c9c583f8e92bb7758ab2ca9a04110c0a1126ec43e5453d196c1"), TWPublicKeyTypeSECP256k1).isValidED25519());
 }
+
+TEST(PublicKeyTests, VerifyRejectsShortDigest) {
+    const auto publicKey = PublicKey(parse_hex("0399c6f51ad6f98c9c583f8e92bb7758ab2ca9a04110c0a1126ec43e5453d196c1"), TWPublicKeyTypeSECP256k1);
+    const auto validSig = parse_hex("0f5d5a9e5fc4b82a625312f3be5d3e8ad017d882de86c72c92fcefa924e894c12071772a14201a3a0debf381b5e8dea39fadb9bcabdc02ee71ab018f55bf717f");
+    const auto shortDigest = parse_hex("0102030405060708");
+
+    EXPECT_FALSE(publicKey.verify(validSig, shortDigest));
+}
+
+TEST(PublicKeyTests, VerifyAsDERRejectsShortDigest) {
+    const auto publicKey = PublicKey(parse_hex("0399c6f51ad6f98c9c583f8e92bb7758ab2ca9a04110c0a1126ec43e5453d196c1"), TWPublicKeyTypeSECP256k1);
+    const auto derSig = parse_hex("304402200f5d5a9e5fc4b82a625312f3be5d3e8ad017d882de86c72c92fcefa924e894c102202071772a14201a3a0debf381b5e8dea39fadb9bcabdc02ee71ab018f55bf717f");
+    const auto shortDigest = parse_hex("0102030405060708");
+
+    EXPECT_FALSE(publicKey.verifyAsDER(derSig, shortDigest));
+}
+
+TEST(PublicKeyTests, VerifyAsDERRejectsInvalidSignatureSize) {
+    const auto publicKey = PublicKey(parse_hex("0399c6f51ad6f98c9c583f8e92bb7758ab2ca9a04110c0a1126ec43e5453d196c1"), TWPublicKeyTypeSECP256k1);
+    const auto digest = parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5");
+
+    EXPECT_FALSE(publicKey.verifyAsDER(Data(), digest));
+    EXPECT_FALSE(publicKey.verifyAsDER(parse_hex("30"), digest));
+    EXPECT_FALSE(publicKey.verifyAsDER(Data(73, 0x30), digest));
+}
+
+TEST(PublicKeyTests, VerifyNist256p1RejectsShortDigest) {
+    const auto privateKey = PrivateKey(parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5"), TWCurveNIST256p1);
+    const auto publicKey = privateKey.getPublicKey(TWPublicKeyTypeNIST256p1);
+    const auto validSig = parse_hex("0f5d5a9e5fc4b82a625312f3be5d3e8ad017d882de86c72c92fcefa924e894c12071772a14201a3a0debf381b5e8dea39fadb9bcabdc02ee71ab018f55bf717f");
+    const auto shortDigest = parse_hex("0102030405060708");
+
+    EXPECT_FALSE(publicKey.verify(validSig, shortDigest));
+}

tests/interface/TWPublicKeyTests.cpp

@@ -91,6 +91,19 @@ TEST(TWPublicKeyTests, VerifyInvalidLength) {
     ASSERT_FALSE(TWPublicKeyVerify(publicKey.get(), signature.get(), message.get()));
 }
 
+TEST(TWPublicKeyTests, VerifyInvalidMessageLength) {
+    const PrivateKey key(parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5"), TWCurveSECP256k1);
+    const auto privateKey = WRAP(TWPrivateKey, new TWPrivateKey{ key });
+
+    // Invalid message length - 31 bytes instead of 32
+    const auto invalidMessage = DATA("de4e9524586d6fce45667f9ff12f661e79870c4105fa0fb58af976619bb114");
+    // Valid 64-byte signature
+    const auto signature = DATA("00000000000000000000000000000000000000000000000000000000000000020123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef80");
+
+    auto publicKey = WRAP(TWPublicKey, TWPrivateKeyGetPublicKeySecp256k1(privateKey.get(), false));
+    ASSERT_FALSE(TWPublicKeyVerify(publicKey.get(), signature.get(), invalidMessage.get()));
+}
+
 TEST(TWPublicKeyTests, VerifyAsDER) {
     const PrivateKey key = PrivateKey(parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5"), TWCurveSECP256k1);
     const auto privateKey = WRAP(TWPrivateKey, new TWPrivateKey{ key });
@@ -108,6 +121,19 @@ TEST(TWPublicKeyTests, VerifyAsDER) {
     ASSERT_FALSE(TWPublicKeyVerify(publicKey.get(), signature.get(), digest.get()));
 }
 
+TEST(TWPublicKeyTests, VerifyAsDERInvalidMessageLength) {
+    const PrivateKey key = PrivateKey(parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5"), TWCurveSECP256k1);
+    const auto privateKey = WRAP(TWPrivateKey, new TWPrivateKey{ key });
+
+    // Invalid message length - 31 bytes instead of 32
+    const auto invalidMessage = DATA("de4e9524586d6fce45667f9ff12f661e79870c4105fa0fb58af976619bb114");
+    // Valid 64-byte signature
+    const auto signature = DATA("00000000000000000000000000000000000000000000000000000000000000020123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef80");
+
+    auto publicKey = WRAP(TWPublicKey, TWPrivateKeyGetPublicKeySecp256k1(privateKey.get(), false));
+    ASSERT_FALSE(TWPublicKeyVerifyAsDER(publicKey.get(), signature.get(), invalidMessage.get()));
+}
+
 TEST(TWPublicKeyTests, VerifyEd25519) {
     const PrivateKey key(parse_hex("afeefca74d9a325cf1d6b6911d61a65c32afa8e02bd5e78e2e4ac2910bab45f5"));
     const auto privateKey = WRAP(TWPrivateKey, new TWPrivateKey{ key });