Fix use-after-free possibility in GetCRLInfo

Author: padelsbach

src/crl.c

@@ -687,13 +687,22 @@ int CheckCertCRL(WOLFSSL_CRL* crl, DecodedCert* cert)
 #ifdef HAVE_CRL_UPDATE_CB
 static void SetCrlInfo(CRL_Entry* entry, CrlInfo *info)
 {
-    info->issuerHash = (byte *)entry->issuerHash;
-    info->issuerHashLen = CRL_DIGEST_SIZE;
-    info->lastDate = (byte *)entry->lastDate;
-    info->lastDateMaxLen = MAX_DATE_SIZE;
+    /* Ensure the copy below stays within bounds. */
+    wc_static_assert(sizeof(info->issuerHashData) == sizeof(entry->issuerHash));
+
+    /* Copy into info's own buffers so the pointers stay valid for the
+     * lifetime of the CrlInfo, not just that of the source entry. */
+    info->issuerHashLen = sizeof(info->issuerHashData);
+    XMEMCPY(info->issuerHashData, entry->issuerHash,
+            sizeof(info->issuerHashData));
+    info->issuerHash = info->issuerHashData;
+    info->lastDateMaxLen = sizeof(info->lastDateData);
+    XMEMCPY(info->lastDateData, entry->lastDate, sizeof(info->lastDateData));
+    info->lastDate = info->lastDateData;
     info->lastDateFormat = entry->lastDateFormat;
-    info->nextDate = (byte *)entry->nextDate;
-    info->nextDateMaxLen = MAX_DATE_SIZE;
+    info->nextDateMaxLen = sizeof(info->nextDateData);
+    XMEMCPY(info->nextDateData, entry->nextDate, sizeof(info->nextDateData));
+    info->nextDate = info->nextDateData;
     info->nextDateFormat = entry->nextDateFormat;
     info->crlNumberSet = entry->crlNumberSet;
     if (info->crlNumberSet)
@@ -702,13 +711,19 @@ static void SetCrlInfo(CRL_Entry* entry, CrlInfo *info)
 
 static void SetCrlInfoFromDecoded(DecodedCRL* entry, CrlInfo *info)
 {
-    info->issuerHash = (byte *)entry->issuerHash;
-    info->issuerHashLen = SIGNER_DIGEST_SIZE;
-    info->lastDate = (byte *)entry->lastDate;
-    info->lastDateMaxLen = MAX_DATE_SIZE;
+    /* Copy into info's own buffers so the pointers stay valid after the
+     * decoded CRL is freed by the caller. */
+    info->issuerHashLen = sizeof(info->issuerHashData);
+    XMEMCPY(info->issuerHashData, entry->issuerHash,
+            sizeof(info->issuerHashData));
+    info->issuerHash = info->issuerHashData;
+    info->lastDateMaxLen = sizeof(info->lastDateData);
+    XMEMCPY(info->lastDateData, entry->lastDate, sizeof(info->lastDateData));
+    info->lastDate = info->lastDateData;
     info->lastDateFormat = entry->lastDateFormat;
-    info->nextDate = (byte *)entry->nextDate;
-    info->nextDateMaxLen = MAX_DATE_SIZE;
+    info->nextDateMaxLen = sizeof(info->nextDateData);
+    XMEMCPY(info->nextDateData, entry->nextDate, sizeof(info->nextDateData));
+    info->nextDate = info->nextDateData;
     info->nextDateFormat = entry->nextDateFormat;
     info->crlNumberSet = entry->crlNumberSet;
     if (info->crlNumberSet)

tests/api.c

@@ -26119,6 +26119,15 @@ static int test_wolfSSL_CTX_LoadCRL_largeCRLnum(void)
         WOLFSSL_SUCCESS);
     AssertIntEQ(XMEMCMP(
         crlInfo.crlNumber, exp_crlnum, XSTRLEN(exp_crlnum)), 0);
+    /* The pointer fields must reference storage inside crlInfo so they stay
+     * valid after the call returns; before the fix they pointed into the
+     * freed decoded CRL. */
+    AssertTrue((byte*)crlInfo.issuerHash >= (byte*)&crlInfo &&
+               (byte*)crlInfo.issuerHash <  (byte*)(&crlInfo + 1));
+    AssertTrue((byte*)crlInfo.lastDate   >= (byte*)&crlInfo &&
+               (byte*)crlInfo.lastDate   <  (byte*)(&crlInfo + 1));
+    AssertTrue((byte*)crlInfo.nextDate   >= (byte*)&crlInfo &&
+               (byte*)crlInfo.nextDate   <  (byte*)(&crlInfo + 1));
     ExpectIntEQ(wolfSSL_CertManagerGetCRLInfo(
         cm, &crlInfo, crlLrgCrlNumBuff, -1, WOLFSSL_FILETYPE_PEM),
         WC_NO_ERR_TRACE(BAD_FUNC_ARG));

wolfssl/ssl.h

@@ -4021,6 +4021,9 @@ typedef struct CrlInfo {
     word32 nextDateMaxLen;
     byte nextDateFormat;
     byte crlNumberSet:1;
+    byte issuerHashData[SIGNER_DIGEST_SIZE];
+    byte lastDateData[MAX_DATE_SIZE];
+    byte nextDateData[MAX_DATE_SIZE];
 } CrlInfo;
 
 typedef void (*CbUpdateCRL)(CrlInfo* old, CrlInfo* cnew);