Merge pull request #365 from padelsbach/wp-finding-255

Fix deep copy in wp_gmac_dup

Author: ColtonWilley

src/wp_gmac.c

@@ -94,6 +94,7 @@ static void wp_gmac_free(wp_GmacCtx* macCtx)
 {
     if (macCtx != NULL) {
         OPENSSL_cleanse(macCtx->key, macCtx->keyLen);
+        OPENSSL_cleanse(macCtx->iv, macCtx->ivLen);
         OPENSSL_clear_free(macCtx->data, macCtx->dataLen);
         OPENSSL_free(macCtx);
     }
@@ -161,9 +162,24 @@ static wp_GmacCtx* wp_gmac_dup(wp_GmacCtx* src)
     }
     if (dst != NULL) {
         *dst = *src;
+        dst->data = NULL;
+        dst->dataLen = 0;
         dst->keyLen = 0;
         dst->ivLen = 0;
 
+        if (src->dataLen != 0) {
+            dst->data = OPENSSL_memdup(src->data, src->dataLen);
+            if (dst->data) {
+                dst->dataLen = src->dataLen;
+            }
+            else {
+                wp_gmac_free(dst);
+                dst = NULL;
+            }
+        }
+    }
+
+    if (dst != NULL) {
         if (src->ivLen != 0) {
             XMEMCPY(dst->iv, src->iv, src->ivLen);
             dst->ivLen = src->ivLen;

test/test_gmac.c

@@ -160,6 +160,118 @@ int test_gmac_create(void *data)
     return ret;
 }
 
-#endif /* WP_HAVE_GMAC */
+int test_gmac_dup(void *data)
+{
+    int ret = 0;
+    EVP_MAC* emac = NULL;
+    EVP_MAC_CTX* src = NULL;
+    EVP_MAC_CTX* dup = NULL;
+    OSSL_PARAM params[4];
+    char cipher[] = "AES-256-GCM";
+    unsigned char key[] = {
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07
+    };
+    unsigned char iv[] = {
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+        0x00, 0x01, 0x02, 0x03
+    };
+    unsigned char prefix[] = "dup-prefix";
+    unsigned char tailA[] = "-tail-a";
+    unsigned char tailB[] = "-tail-b";
+    unsigned char msgA[sizeof(prefix) + sizeof(tailA)];
+    unsigned char msgB[sizeof(prefix) + sizeof(tailB)];
+    unsigned char macA[16];
+    unsigned char macB[16];
+    unsigned char expA[16];
+    unsigned char expB[16];
+    size_t macASz = sizeof(macA);
+    size_t macBSz = sizeof(macB);
+    int expASz = sizeof(expA);
+    int expBSz = sizeof(expB);
+
+    (void)data;
+
+    /* Build full messages used for one-shot expected MAC calculations. */
+    XMEMCPY(msgA, prefix, sizeof(prefix));
+    XMEMCPY(msgA + sizeof(prefix), tailA, sizeof(tailA));
+    XMEMCPY(msgB, prefix, sizeof(prefix));
+    XMEMCPY(msgB + sizeof(prefix), tailB, sizeof(tailB));
+
+    /* Compute expected MACs for each post-duplication branch. */
+    ret = test_gmac_gen_mac(wpLibCtx, cipher, iv, (int)sizeof(iv), key,
+        (int)sizeof(key), msgA, (int)sizeof(msgA), expA, &expASz);
+    if (ret != 0) {
+        PRINT_MSG("Generate expected MAC A failed");
+    }
+    if (ret == 0) {
+        ret = test_gmac_gen_mac(wpLibCtx, cipher, iv, (int)sizeof(iv),
+            key, (int)sizeof(key), msgB, (int)sizeof(msgB), expB, &expBSz);
+        if (ret != 0) {
+            PRINT_MSG("Generate expected MAC B failed");
+        }
+    }
+
+    params[0] = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_CIPHER,
+        cipher, 0);
+    params[1] = OSSL_PARAM_construct_octet_string(OSSL_MAC_PARAM_KEY,
+        (void*)key, sizeof(key));
+    params[2] = OSSL_PARAM_construct_octet_string(OSSL_MAC_PARAM_IV,
+        (void*)iv, sizeof(iv));
+    params[3] = OSSL_PARAM_construct_end();
+
+    if (ret == 0) {
+        ret = (emac = EVP_MAC_fetch(wpLibCtx, "GMAC", NULL)) == NULL;
+    }
+    if (ret == 0) {
+        ret = (src = EVP_MAC_CTX_new(emac)) == NULL;
+    }
+    if (ret == 0) {
+        ret = EVP_MAC_CTX_set_params(src, params) != 1;
+    }
+    if (ret == 0) {
+        ret = EVP_MAC_init(src, NULL, 0, NULL) != 1;
+    }
+    if (ret == 0) {
+        ret = EVP_MAC_update(src, prefix, sizeof(prefix)) != 1;
+    }
+    /* Duplicate after partial update so both contexts start from same state. */
+    if (ret == 0) {
+        ret = (dup = EVP_MAC_CTX_dup(src)) == NULL;
+    }
+    if (ret == 0) {
+        ret = EVP_MAC_update(src, tailA, sizeof(tailA)) != 1;
+    }
+    if (ret == 0) {
+        ret = EVP_MAC_update(dup, tailB, sizeof(tailB)) != 1;
+    }
+    if (ret == 0) {
+        ret = EVP_MAC_final(src, macA, &macASz, sizeof(macA)) != 1;
+    }
+    if (ret == 0) {
+        ret = EVP_MAC_final(dup, macB, &macBSz, sizeof(macB)) != 1;
+    }
+    /* Verify each branch matches its independently generated expected MAC. */
+    if (ret == 0) {
+        if ((macASz != (size_t)expASz) || (memcmp(macA, expA, macASz) != 0)) {
+            PRINT_MSG("Duplicated source context MAC mismatch");
+            ret = -1;
+        }
+    }
+    if (ret == 0) {
+        if ((macBSz != (size_t)expBSz) || (memcmp(macB, expB, macBSz) != 0)) {
+            PRINT_MSG("Duplicated destination context MAC mismatch");
+            ret = -1;
+        }
+    }
 
+    EVP_MAC_CTX_free(dup);
+    EVP_MAC_CTX_free(src);
+    EVP_MAC_free(emac);
 
+    return ret;
+}
+
+#endif /* WP_HAVE_GMAC */

test/unit.c

@@ -209,6 +209,7 @@ TEST_CASE test_case[] = {
 #endif
 #ifdef WP_HAVE_GMAC
     TEST_DECL(test_gmac_create, &flags),
+    TEST_DECL(test_gmac_dup, &flags),
 #endif
 #ifdef WP_HAVE_TLS1_PRF
     TEST_DECL(test_tls1_prf, NULL),
@@ -813,4 +814,3 @@ int main(int argc, char* argv[])
 
     return err;
 }
-

test/unit.h

@@ -123,6 +123,7 @@ int test_cmac_create(void *data);
 
 #ifdef WP_HAVE_GMAC
 int test_gmac_create(void *data);
+int test_gmac_dup(void *data);
 #endif /* WP_HAVE_GMAC */
 
 #ifdef WP_HAVE_TLS1_PRF