Merge bitcoin-core/secp256k1#990: Add comment on length checks when parsing ECDSA sigs

real-or-random · real-or-random · commit 5e9a4d7aecad · 2023-12-07T09:26:38.000+01:00
e02f313 Add comment on length checks when parsing ECDSA sigs (Tim Ruffing) Pull request description: I claim the check can be removed but I don't want to touch this stable and well-tested code. On the way, we fix grammar in another comment. ACKs for top commit: sipa: ACK e02f313 RandyMcMillan: ACK e02f313 Tree-SHA512: f82691a8f5db82a1e9683e52ce8e952ebd56b476a2817c5a876ce4638254b7b4ac93175318fb59598ed5532f33433951d75afea03724ef4419c3e1bd12ca8c20
diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h
@@ -66,8 +66,7 @@ static int secp256k1_der_read_len(size_t *len, const unsigned char **sigp, const
     }
     if (lenleft > sizeof(size_t)) {
         /* The resulting length would exceed the range of a size_t, so
-         * certainly longer than the passed array size.
-         */
+         * it is certainly longer than the passed array size. */
         return 0;
     }
     while (lenleft > 0) {
@@ -76,7 +75,9 @@ static int secp256k1_der_read_len(size_t *len, const unsigned char **sigp, const
         lenleft--;
     }
     if (*len > (size_t)(sigend - *sigp)) {
-        /* Result exceeds the length of the passed array. */
+        /* Result exceeds the length of the passed array.
+           (Checking this is the responsibility of the caller but it
+           can't hurt do it here, too.) */
         return 0;
     }
     if (*len < 128) {

Original file line number	Diff line number	Diff line change
`@@ -66,8 +66,7 @@ static int secp256k1_der_read_len(size_t len, const unsigned char *sigp, const`
`66`	`66`	`}`
`67`	`67`	`if (lenleft > sizeof(size_t)) {`
`68`	`68`	`/* The resulting length would exceed the range of a size_t, so`
`69`		`- * certainly longer than the passed array size.`
`70`		`- */`
	`69`	`+ * it is certainly longer than the passed array size. */`
`71`	`70`	`return 0;`
`72`	`71`	`}`
`73`	`72`	`while (lenleft > 0) {`
`@@ -76,7 +75,9 @@ static int secp256k1_der_read_len(size_t len, const unsigned char *sigp, const`
`76`	`75`	`lenleft--;`
`77`	`76`	`}`
`78`	`77`	`if (len > (size_t)(sigend - sigp)) {`
`79`		`- /* Result exceeds the length of the passed array. */`
	`78`	`+ /* Result exceeds the length of the passed array.`
	`79`	`+ (Checking this is the responsibility of the caller but it`
	`80`	`+ can't hurt do it here, too.) */`
`80`	`81`	`return 0;`
`81`	`82`	`}`
`82`	`83`	`if (*len < 128) {`