Merge bitcoin-core/secp256k1#1928: tests: add coverage for exact-size DER signature serialization
What the developer wrote
Authored by merge-script
Merge bitcoin-core/secp256k1#1928: tests: add coverage for exact-size DER signature serialization
500ccb60f70550332adf6b6f894ce84659cd6831 tests: add coverage for exact-size DER signature serialization (Bruno Garcia)
Pull request description:
The existing tests only serialize into oversized buffers or one that is far too small, so the boundary case where the output buffer is exactly the required size was untested. Check that this succeeds with identical output, and that one byte less fails while reporting the required size.
Kills the following mutant (https://secp256k1.space/src/ecdsa_impl.h#1141):
```diff
diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h
index 5963877..499fe03 100644
--- a/src/ecdsa_impl.h
+++ b/src/ecdsa_impl.h
@@ -176,7 +176,7 @@ static int secp256k1_ecdsa_sig_serialize(unsigned char *sig, size_t *size, const
secp256k1_scalar_get_b32(&s[1], as);
while (lenR > 1 && rp[0] == 0 && rp[1] < 0x80) { lenR--; rp++; }
while (lenS > 1 && sp[0] == 0 && sp[1] < 0x80) { lenS--; sp++; }
- if (*size < 6+lenS+lenR) {
+ if (*size <= 6+lenS+lenR) {
*size = 6 + lenS + lenR;
return 0;
}
```
ACKs for top commit:
real-or-random:
utACK 500ccb60f70550332adf6b6f894ce84659cd6831
theStack:
ACK 500ccb60f70550332adf6b6f894ce84659cd6831
Tree-SHA512: 8ac002a9e67729f5cb09a9ba8244e926e81898abb273ae4f43a13ccf981322e434caee06d27dcae9b840e0b850908c2281399d48a444319bcc11a0af82bb52a3
What changed, and why it matters
This commit only adds new test code to check that a specific function behaves correctly when given a buffer of exactly the right size. It does not change any production code, fix a bug, or introduce a vulnerability. It is a routine improvement to the test suite.
No security action needed. This is a benign test-only change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit adds test coverage in src/tests.c for secp256k1_ecdsa_signature_serialize_der. It verifies that serializing a DER signature into a buffer of exactly the required length succeeds and produces identical output, and that a buffer one byte shorter fails and reports the required size. The change is purely additive to tests and references a surviving mutation in src/ecdsa_impl.h that the new test would catch. No library code is modified.
Changed components
src/tests.cInspect captured patch +12 / −0
### src/tests.c
@@ -6828,6 +6828,18 @@ static void test_ecdsa_end_to_end(void) {
memset(&signature[0], 0, sizeof(signature[0]));
CHECK(secp256k1_ecdsa_signature_parse_der(CTX, &signature[0], sig, siglen) == 1);
CHECK(secp256k1_ecdsa_verify(CTX, &signature[0], message, &pubkey) == 1);
+ /* Serializing into a buffer of exactly the required size succeeds and
+ * yields the same encoding; one byte less fails and reports the size. */
+ {
+ unsigned char sig2[74];
+ size_t siglen2 = siglen;
+ CHECK(secp256k1_ecdsa_signature_serialize_der(CTX, sig2, &siglen2, &signature[0]) == 1);
+ CHECK(siglen2 == siglen);
+ CHECK(secp256k1_memcmp_var(sig2, sig, siglen) == 0);
+ siglen2 = siglen - 1;
+ CHECK(secp256k1_ecdsa_signature_serialize_der(CTX, sig2, &siglen2, &signature[0]) == 0);
+ CHECK(siglen2 == siglen);
+ }
/* Serialize/destroy/parse DER and verify again. */
siglen = 74;
CHECK(secp256k1_ecdsa_signature_serialize_der(CTX, sig, &siglen, &signature[0]) == 1);Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.