Parse DER-enconded length into a size_t instead of an int

real-or-random · real-or-random · commit 01ee1b3b3c66 · 2019-05-23T15:22:29.000+02:00
This avoids a possibly implementation-defined signed (int) to unsigned
(size_t) conversion portably.
diff --git a/src/ecdsa_impl.h b/src/ecdsa_impl.h
@@ -46,68 +46,73 @@ static const secp256k1_fe secp256k1_ecdsa_const_p_minus_order = SECP256K1_FE_CON
     0, 0, 0, 1, 0x45512319UL, 0x50B75FC4UL, 0x402DA172UL, 0x2FC9BAEEUL
 );
 
-static int secp256k1_der_read_len(const unsigned char **sigp, const unsigned char *sigend) {
-    int lenleft, b1;
-    size_t ret = 0;
+static int secp256k1_der_read_len(size_t *len, const unsigned char **sigp, const unsigned char *sigend) {
+    size_t lenleft;
+    unsigned char b1;
+    VERIFY_CHECK(len != NULL);
+    *len = 0;
     if (*sigp >= sigend) {
-        return -1;
+        return 0;
     }
     b1 = *((*sigp)++);
     if (b1 == 0xFF) {
         /* X.690-0207 8.1.3.5.c the value 0xFF shall not be used. */
-        return -1;
+        return 0;
     }
     if ((b1 & 0x80) == 0) {
         /* X.690-0207 8.1.3.4 short form length octets */
-        return b1;
+        *len = b1;
+        return 1;
     }
     if (b1 == 0x80) {
         /* Indefinite length is not allowed in DER. */
-        return -1;
+        return 0;
     }
     /* X.690-207 8.1.3.5 long form length octets */
     lenleft = b1 & 0x7F; /* lenleft is at least 1 */
-    if (lenleft > sigend - *sigp) {
-        return -1;
+    if (lenleft > (size_t)(sigend - *sigp)) {
+        return 0;
     }
     if (**sigp == 0) {
         /* Not the shortest possible length encoding. */
-        return -1;
+        return 0;
     }
-    if ((size_t)lenleft > sizeof(size_t)) {
+    if (lenleft > sizeof(size_t)) {
         /* The resulting length would exceed the range of a size_t, so
          * certainly longer than the passed array size.
          */
-        return -1;
+        return 0;
     }
     while (lenleft > 0) {
-        ret = (ret << 8) | **sigp;
+        *len = (*len << 8) | **sigp;
         (*sigp)++;
         lenleft--;
     }
-    if (ret > (size_t)(sigend - *sigp)) {
+    if (*len > (size_t)(sigend - *sigp)) {
         /* Result exceeds the length of the passed array. */
-        return -1;
+        return 0;
     }
-    if (ret < 128) {
+    if (*len < 128) {
         /* Not the shortest possible length encoding. */
-        return -1;
+        return 0;
     }
-    return ret;
+    return 1;
 }
 
 static int secp256k1_der_parse_integer(secp256k1_scalar *r, const unsigned char **sig, const unsigned char *sigend) {
     int overflow = 0;
     unsigned char ra[32] = {0};
-    int rlen;
+    size_t rlen;
 
     if (*sig == sigend || **sig != 0x02) {
         /* Not a primitive integer (X.690-0207 8.3.1). */
         return 0;
     }
     (*sig)++;
-    rlen = secp256k1_der_read_len(sig, sigend);
-    if (rlen <= 0 || (*sig) + rlen > sigend) {
+    if (secp256k1_der_read_len(&rlen, sig, sigend) == 0) {
+        return 0;
+    }
+    if (rlen == 0 || *sig + rlen > sigend) {
         /* Exceeds bounds or not at least length 1 (X.690-0207 8.3.1).  */
         return 0;
     }
@@ -144,13 +149,15 @@ static int secp256k1_der_parse_integer(secp256k1_scalar *r, const unsigned char
 
 static int secp256k1_ecdsa_sig_parse(secp256k1_scalar *rr, secp256k1_scalar *rs, const unsigned char *sig, size_t size) {
     const unsigned char *sigend = sig + size;
-    int rlen;
+    size_t rlen;
     if (sig == sigend || *(sig++) != 0x30) {
         /* The encoding doesn't start with a constructed sequence (X.690-0207 8.9.1). */
         return 0;
     }
-    rlen = secp256k1_der_read_len(&sig, sigend);
-    if (rlen < 0 || sig + rlen > sigend) {
+    if (secp256k1_der_read_len(&rlen, &sig, sigend) == 0) {
+        return 0;
+    }
+    if (sig + rlen > sigend) {
         /* Tuple exceeds bounds */
         return 0;
     }
