diff --git a/crypto/ecdsa.c b/crypto/ecdsa.c index 023194d92..1ea4e59eb 100644 --- a/crypto/ecdsa.c +++ b/crypto/ecdsa.c @@ -1204,8 +1204,8 @@ int ecdsa_sig_to_der(const uint8_t *sig, uint8_t *der) { return *len + 2; } -// Parse a DER-encoded signature. We don't check whether the encoded integers -// satisfy DER requirements regarding leading zeros. +// Parse a DER-encoded signature. We check whether the encoded integers satisfy +// DER requirements regarding leading zeros. int ecdsa_sig_from_der(const uint8_t *der, size_t der_len, uint8_t sig[64]) { memzero(sig, 64); @@ -1229,10 +1229,20 @@ int ecdsa_sig_from_der(const uint8_t *der, size_t der_len, uint8_t sig[64]) { return 1; } - // Skip a possible leading zero. - if (int_len != 0 && der[pos] == 0) { + // Positive integers must not start with an octet that has bit 8 set to 1. + if (int_len == 0 || der[pos] > 0x7f) { + return 1; + } + + // Skip a possible leading null octet. + if (int_len > 1 && der[pos] == 0x00) { int_len--; pos++; + + // Check that integer uses the shortest possible encoding. + if (der[pos] < 0x80) { + return 1; + } } // Copy the integer to the output, making sure it fits. diff --git a/crypto/tests/test_check.c b/crypto/tests/test_check.c index 1ff4b3bc4..5a9a07eab 100644 --- a/crypto/tests/test_check.c +++ b/crypto/tests/test_check.c @@ -6776,6 +6776,13 @@ START_TEST(test_ecdsa_der) { "0000000000000000000000000000000000000000000000000000000000000000", "3006020100020100", }, + {NULL, NULL, "3008020200ee020200ff00"}, + {NULL, NULL, + "304402207f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1" + "f0220800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f"}, + {NULL, NULL, + "30440220007f0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1" + "e022000800102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e"}, }; uint8_t sig[64]; @@ -6784,12 +6791,16 @@ START_TEST(test_ecdsa_der) { for (size_t i = 0; i < (sizeof(vectors) / sizeof(*vectors)); ++i) { size_t der_len = strlen(vectors[i].der) / 2; memcpy(der, fromhex(vectors[i].der), der_len); - memcpy(sig, fromhex(vectors[i].r), 32); - memcpy(sig + 32, fromhex(vectors[i].s), 32); - ck_assert_int_eq(ecdsa_sig_to_der(sig, out), der_len); - ck_assert_mem_eq(out, der, der_len); - ck_assert_int_eq(ecdsa_sig_from_der(der, der_len, out), 0); - ck_assert_mem_eq(out, sig, 64); + if (vectors[i].r != NULL) { + memcpy(sig, fromhex(vectors[i].r), 32); + memcpy(sig + 32, fromhex(vectors[i].s), 32); + ck_assert_int_eq(ecdsa_sig_to_der(sig, out), der_len); + ck_assert_mem_eq(out, der, der_len); + ck_assert_int_eq(ecdsa_sig_from_der(der, der_len, out), 0); + ck_assert_mem_eq(out, sig, 64); + } else { + ck_assert_int_eq(ecdsa_sig_from_der(der, der_len, out), 1); + } } } END_TEST