Merge pull request #10025 from embhorn/zd21392

Fix DecodeObjectId unknown ext parse

Author: dgarske

tests/api/test_asn.c

@@ -1028,3 +1028,77 @@ int test_DecodeAltNames_length_underflow(void)
 #endif /* !NO_CERTS && !NO_RSA && !NO_ASN */
     return EXPECT_RESULT();
 }
+
+int test_wc_DecodeObjectId(void)
+{
+    EXPECT_DECLS;
+
+#if !defined(NO_ASN) && \
+    (defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT))
+    {
+        /* OID 1.2.840.113549.1.1.11 (sha256WithRSAEncryption)
+         * DER encoding: 2a 86 48 86 f7 0d 01 01 0b
+         * First byte 0x2a = 42 => arc0 = 42/40 = 1, arc1 = 42%40 = 2
+         * Remaining arcs: 840, 113549, 1, 1, 11
+         */
+        static const byte oid_sha256rsa[] = {
+            0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b
+        };
+        word16 out[MAX_OID_SZ];
+        word32 outSz;
+
+        /* Test 1: Normal decode */
+        outSz = MAX_OID_SZ;
+        ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa),
+                                   out, &outSz), 0);
+        ExpectIntEQ((int)outSz, 7);
+        ExpectIntEQ(out[0], 1);
+        ExpectIntEQ(out[1], 2);
+        ExpectIntEQ(out[2], 840);
+        ExpectIntEQ(out[3], (word16)113549); /* truncated to word16 */
+        ExpectIntEQ(out[4], 1);
+        ExpectIntEQ(out[5], 1);
+        ExpectIntEQ(out[6], 11);
+
+        /* Test 2: NULL args */
+        outSz = MAX_OID_SZ;
+        ExpectIntEQ(DecodeObjectId(NULL, sizeof(oid_sha256rsa), out, &outSz),
+                    WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+        ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa),
+                                   out, NULL),
+                    WC_NO_ERR_TRACE(BAD_FUNC_ARG));
+
+        /* Test 3 (Bug 1): outSz=1 must return BUFFER_E, not OOB write.
+         * The first OID byte decodes into two arcs, so outSz must be >= 2. */
+        outSz = 1;
+        ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa),
+                                   out, &outSz),
+                    WC_NO_ERR_TRACE(BUFFER_E));
+
+        /* Test 4: outSz=0 must also return BUFFER_E */
+        outSz = 0;
+        ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa),
+                                   out, &outSz),
+                    WC_NO_ERR_TRACE(BUFFER_E));
+
+        /* Test 5: outSz=2 is enough for a single-byte OID (two arcs) */
+        {
+            static const byte oid_one_byte[] = { 0x2a }; /* 1.2 */
+            outSz = 2;
+            ExpectIntEQ(DecodeObjectId(oid_one_byte, sizeof(oid_one_byte),
+                                       out, &outSz), 0);
+            ExpectIntEQ((int)outSz, 2);
+            ExpectIntEQ(out[0], 1);
+            ExpectIntEQ(out[1], 2);
+        }
+
+        /* Test 6: Buffer too small for later arcs */
+        outSz = 3; /* only room for 3 arcs, but OID has 7 */
+        ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa),
+                                   out, &outSz),
+                    WC_NO_ERR_TRACE(BUFFER_E));
+    }
+#endif /* !NO_ASN && (HAVE_OID_DECODING || WOLFSSL_ASN_PRINT) */
+
+    return EXPECT_RESULT();
+}

tests/api/test_asn.h

@@ -30,13 +30,15 @@ int test_wc_IndexSequenceOf(void);
 int test_wolfssl_local_MatchBaseName(void);
 int test_wc_DecodeRsaPssParams(void);
 int test_DecodeAltNames_length_underflow(void);
+int test_wc_DecodeObjectId(void);
 
 #define TEST_ASN_DECLS                                              \
     TEST_DECL_GROUP("asn", test_SetAsymKeyDer),                     \
     TEST_DECL_GROUP("asn", test_GetSetShortInt),                    \
     TEST_DECL_GROUP("asn", test_wc_IndexSequenceOf),                \
     TEST_DECL_GROUP("asn", test_wolfssl_local_MatchBaseName),       \
     TEST_DECL_GROUP("asn", test_wc_DecodeRsaPssParams),             \
-    TEST_DECL_GROUP("asn", test_DecodeAltNames_length_underflow)
+    TEST_DECL_GROUP("asn", test_DecodeAltNames_length_underflow),    \
+    TEST_DECL_GROUP("asn", test_wc_DecodeObjectId)
 
 #endif /* WOLFCRYPT_TEST_ASN_H */

wolfcrypt/src/asn.c

@@ -6895,15 +6895,18 @@ int DecodeObjectId(const byte* in, word32 inSz, word16* out, word32* outSz)
         t = (t << 7) | (in[x] & 0x7F);
         cnt++;
         if (!(in[x] & 0x80)) {
-            if (y >= (int)*outSz) {
-                return BUFFER_E;
-            }
             if (y == 0) {
+                if ((int)*outSz < 2) {
+                    return BUFFER_E;
+                }
                 out[0] = (word16)(t / 40);
                 out[1] = (word16)(t % 40);
                 y = 2;
             }
             else {
+                if (y >= (int)*outSz) {
+                    return BUFFER_E;
+                }
                 out[y++] = (word16)t;
             }
             t = 0; /* reset tmp */
@@ -7000,7 +7003,7 @@ static int DumpOID(const byte* oidData, word32 oidSz, word32 oid,
     #ifdef HAVE_OID_DECODING
     {
         word16 decOid[MAX_OID_SZ];
-        word32 decOidSz = sizeof(decOid);
+        word32 decOidSz = MAX_OID_SZ;
         /* Decode the OID into dotted form. */
         ret = DecodeObjectId(oidData, oidSz, decOid, &decOidSz);
         if (ret == 0) {
@@ -20023,7 +20026,7 @@ static int DecodeCertExtensions(DecodedCert* cert)
             if (isUnknownExt && (cert->unknownExtCallback != NULL ||
                                  cert->unknownExtCallbackEx != NULL)) {
                 word16 decOid[MAX_OID_SZ];
-                word32 decOidSz = sizeof(decOid);
+                word32 decOidSz = MAX_OID_SZ;
                 ret = DecodeObjectId(
                           dataASN[CERTEXTASN_IDX_OID].data.oid.data,
                           dataASN[CERTEXTASN_IDX_OID].data.oid.length,

wolfssl/wolfcrypt/asn.h

@@ -2247,6 +2247,10 @@ typedef enum MimeStatus
     #define SetAlgoID wc_SetAlgoID
     #define SetAsymKeyDer wc_SetAsymKeyDer
     #define CalcHashId wc_CalcHashId
+    #if defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT) || \
+        defined(OPENSSL_ALL)
+        #define DecodeObjectId wc_DecodeObjectId
+    #endif
     #if defined(WOLFSSL_AKID_NAME) && !defined(GetCAByAKID)
         /* GetCAByAKID() has two implementations, a full implementation in
          * src/ssl.c, and a dummy implementation in wolfcrypt/src/asn.c for
@@ -2489,7 +2493,7 @@ WOLFSSL_LOCAL word32 wc_oid_sum(const byte* input, int length);
 #endif
 #if defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT) || \
     defined(OPENSSL_ALL)
-    WOLFSSL_LOCAL int DecodeObjectId(const byte* in, word32 inSz,
+    WOLFSSL_TEST_VIS int DecodeObjectId(const byte* in, word32 inSz,
         word16* out, word32* outSz);
 #endif
 WOLFSSL_LOCAL int GetASNObjectId(const byte* input, word32* inOutIdx, int* len,