Fix Use after free when copying cipher ctx
authorShane Lontis <shane.lontis@oracle.com>
Mon, 18 Nov 2019 03:13:05 +0000 (13:13 +1000)
committerShane Lontis <shane.lontis@oracle.com>
Mon, 18 Nov 2019 03:13:05 +0000 (13:13 +1000)
Fixes #10438
issue found by clusterfuzz/ossfuzz

The dest was getting a copy of the src structure which contained a pointer that should point to an offset inside itself - because of the copy it was pointing to the original structure.

The setup for a ctx is mainly done by the initkey method in the PROV_CIPHER_HW structure. Because of this it makes sense that the structure should also contain a copyctx method that is use to resolve any pointers that need to be setup.

A dup_ctx has been added to the cipher_enc tests in evp_test. It does a dup after setup and then frees the original ctx. This detects any floating pointers in the duplicated context that were pointing back to the freed ctx.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10443)

17 files changed:
providers/common/include/prov/ciphercommon.h
providers/implementations/ciphers/cipher_aes.c
providers/implementations/ciphers/cipher_aes_hw.c
providers/implementations/ciphers/cipher_aes_hw_aesni.inc
providers/implementations/ciphers/cipher_aes_hw_s390x.inc
providers/implementations/ciphers/cipher_aes_hw_t4.inc
providers/implementations/ciphers/cipher_aes_ocb.c
providers/implementations/ciphers/cipher_aes_xts.c
providers/implementations/ciphers/cipher_aes_xts_hw.c
providers/implementations/ciphers/cipher_aria.c
providers/implementations/ciphers/cipher_aria_hw.c
providers/implementations/ciphers/cipher_camellia.c
providers/implementations/ciphers/cipher_camellia_hw.c
providers/implementations/ciphers/cipher_camellia_hw_t4.inc
providers/implementations/ciphers/cipher_sm4.c
providers/implementations/ciphers/cipher_sm4_hw.c
test/evp_test.c

index 2f77f48..c9b0034 100644 (file)
@@ -68,6 +68,7 @@ struct prov_cipher_ctx_st {
 struct prov_cipher_hw_st {
     int (*init)(PROV_CIPHER_CTX *dat, const uint8_t *key, size_t keylen);
     PROV_CIPHER_HW_FN *cipher;
+    void (*copyctx)(PROV_CIPHER_CTX *dst, const PROV_CIPHER_CTX *src);
 };
 
 OSSL_OP_cipher_encrypt_init_fn cipher_generic_einit;
@@ -233,6 +234,16 @@ static int cipher_hw_##NAME##_##MODE##_cipher(PROV_CIPHER_CTX *ctx,            \
     return 1;                                                                  \
 }
 
+#define IMPLEMENT_CIPHER_HW_COPYCTX(name, CTX_TYPE)                            \
+static void name(PROV_CIPHER_CTX *dst, const PROV_CIPHER_CTX *src)             \
+{                                                                              \
+    CTX_TYPE *sctx = (CTX_TYPE *)src;                                          \
+    CTX_TYPE *dctx = (CTX_TYPE *)dst;                                          \
+                                                                               \
+    *dctx = *sctx;                                                             \
+    dst->ks = &dctx->ks.ks;                                                    \
+}
+
 #define CIPHER_DEFAULT_GETTABLE_CTX_PARAMS_START(name)                         \
 static const OSSL_PARAM name##_known_gettable_ctx_params[] = {                 \
     OSSL_PARAM_size_t(OSSL_CIPHER_PARAM_KEYLEN, NULL),                         \
index c62d2d2..561377a 100644 (file)
@@ -31,7 +31,7 @@ static void *aes_dupctx(void *ctx)
         ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    *ret = *in;
+    in->base.hw->copyctx(&ret->base, &in->base);
 
     return ret;
 }
index 8519662..bc733ee 100644 (file)
@@ -106,10 +106,13 @@ static int cipher_hw_aes_initkey(PROV_CIPHER_CTX *dat,
     return 1;
 }
 
+IMPLEMENT_CIPHER_HW_COPYCTX(cipher_hw_aes_copyctx, PROV_AES_CTX)
+
 #define PROV_CIPHER_HW_aes_mode(mode)                                          \
 static const PROV_CIPHER_HW aes_##mode = {                                     \
     cipher_hw_aes_initkey,                                                     \
-    cipher_hw_generic_##mode                                                   \
+    cipher_hw_generic_##mode,                                                  \
+    cipher_hw_aes_copyctx                                                      \
 };                                                                             \
 PROV_CIPHER_HW_declare(mode)                                                   \
 const PROV_CIPHER_HW *PROV_CIPHER_HW_aes_##mode(size_t keybits)                \
index 6070939..8d3aef6 100644 (file)
@@ -76,7 +76,8 @@ static int cipher_hw_aesni_ecb(PROV_CIPHER_CTX *ctx, unsigned char *out,
 #define PROV_CIPHER_HW_declare(mode)                                           \
 static const PROV_CIPHER_HW aesni_##mode = {                                   \
     cipher_hw_aesni_initkey,                                                   \
-    cipher_hw_aesni_##mode                                                     \
+    cipher_hw_aesni_##mode,                                                    \
+    cipher_hw_aes_copyctx                                                      \
 };
 #define PROV_CIPHER_HW_select(mode)                                            \
 if (AESNI_CAPABLE)                                                             \
index 805fa91..f60ca9c 100644 (file)
@@ -194,6 +194,7 @@ static int s390x_aes_cfb8_cipher_hw(PROV_CIPHER_CTX *dat, unsigned char *out,
 static const PROV_CIPHER_HW s390x_aes_##mode = {                               \
     s390x_aes_##mode##_initkey,                                                \
     s390x_aes_##mode##_cipher_hw                                               \
+    cipher_hw_aes_copyctx                                                      \
 };
 #define PROV_CIPHER_HW_select(mode)                                            \
 if ((keybits == 128 && S390X_aes_128_##mode##_CAPABLE)                         \
index 21b6727..07b6578 100644 (file)
@@ -88,7 +88,8 @@ static int cipher_hw_aes_t4_initkey(PROV_CIPHER_CTX *dat,
 #define PROV_CIPHER_HW_declare(mode)                                           \
 static const PROV_CIPHER_HW aes_t4_##mode = {                                  \
     cipher_hw_aes_t4_initkey,                                                  \
-    cipher_hw_generic_##mode                                                   \
+    cipher_hw_generic_##mode,                                                  \
+    cipher_hw_aes_copyctx                                                      \
 };
 #define PROV_CIPHER_HW_select(mode)                                            \
     if (SPARC_AES_CAPABLE)                                                     \
index d30a666..ba4e4f2 100644 (file)
@@ -84,8 +84,8 @@ static ossl_inline int aes_generic_ocb_cipher(PROV_AES_OCB_CTX *ctx,
 static ossl_inline int aes_generic_ocb_copy_ctx(PROV_AES_OCB_CTX *dst,
                                                 PROV_AES_OCB_CTX *src)
 {
-    return (!CRYPTO_ocb128_copy_ctx(&dst->ocb, &src->ocb,
-                                    &src->ksenc.ks, &src->ksdec.ks));
+    return CRYPTO_ocb128_copy_ctx(&dst->ocb, &src->ocb,
+                                  &dst->ksenc.ks, &dst->ksdec.ks);
 }
 
 /*-
index 2ad1fbe..d60e08e 100644 (file)
@@ -134,7 +134,7 @@ static void *aes_xts_dupctx(void *vctx)
         ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    *ret = *in;
+    in->base.hw->copyctx(&ret->base, &in->base);
     return ret;
 }
 
index 9ac70c4..b9472b2 100644 (file)
@@ -76,6 +76,17 @@ static int cipher_hw_aes_xts_generic_initkey(PROV_CIPHER_CTX *ctx,
     return 1;
 }
 
+static void cipher_hw_aes_xts_copyctx(PROV_CIPHER_CTX *dst,
+                                      const PROV_CIPHER_CTX *src)
+{
+    PROV_AES_XTS_CTX *sctx = (PROV_AES_XTS_CTX *)src;
+    PROV_AES_XTS_CTX *dctx = (PROV_AES_XTS_CTX *)dst;
+
+    *dctx = *sctx;
+    dctx->xts.key1 = &dctx->ks1.ks;
+    dctx->xts.key2 = &dctx->ks2.ks;
+}
+
 #if defined(AESNI_CAPABLE)
 
 static int cipher_hw_aesni_xts_initkey(PROV_CIPHER_CTX *ctx,
@@ -92,7 +103,8 @@ static int cipher_hw_aesni_xts_initkey(PROV_CIPHER_CTX *ctx,
 # define PROV_CIPHER_HW_declare_xts()                                          \
 static const PROV_CIPHER_HW aesni_xts = {                                      \
     cipher_hw_aesni_xts_initkey,                                               \
-    NULL                                                                       \
+    NULL,                                                                      \
+    cipher_hw_aes_xts_copyctx                                                  \
 };
 # define PROV_CIPHER_HW_select_xts()                                           \
 if (AESNI_CAPABLE)                                                             \
@@ -130,7 +142,8 @@ static int cipher_hw_aes_xts_t4_initkey(PROV_CIPHER_CTX *ctx,
 # define PROV_CIPHER_HW_declare_xts()                                          \
 static const PROV_CIPHER_HW aes_xts_t4 = {                                     \
     cipher_hw_aes_xts_t4_initkey,                                              \
-    NULL                                                                       \
+    NULL,                                                                      \
+    cipher_hw_aes_xts_copyctx                                                  \
 };
 # define PROV_CIPHER_HW_select_xts()                                           \
 if (SPARC_AES_CAPABLE)                                                         \
@@ -143,7 +156,8 @@ if (SPARC_AES_CAPABLE)                                                         \
 
 static const PROV_CIPHER_HW aes_generic_xts = {
     cipher_hw_aes_xts_generic_initkey,
-    NULL
+    NULL,
+    cipher_hw_aes_xts_copyctx
 };
 PROV_CIPHER_HW_declare_xts()
 const PROV_CIPHER_HW *PROV_CIPHER_HW_aes_xts(size_t keybits)
index da86401..37afa06 100644 (file)
@@ -31,7 +31,7 @@ static void *aria_dupctx(void *ctx)
         ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    *ret = *in;
+    in->base.hw->copyctx(&ret->base, &in->base);
 
     return ret;
 }
index b644be8..b0c5675 100644 (file)
@@ -29,10 +29,13 @@ static int cipher_hw_aria_initkey(PROV_CIPHER_CTX *dat,
     return 1;
 }
 
+IMPLEMENT_CIPHER_HW_COPYCTX(cipher_hw_aria_copyctx, PROV_ARIA_CTX)
+
 # define PROV_CIPHER_HW_aria_mode(mode)                                        \
 static const PROV_CIPHER_HW aria_##mode = {                                    \
     cipher_hw_aria_initkey,                                                    \
-    cipher_hw_chunked_##mode                                                   \
+    cipher_hw_chunked_##mode,                                                  \
+    cipher_hw_aria_copyctx                                                     \
 };                                                                             \
 const PROV_CIPHER_HW *PROV_CIPHER_HW_aria_##mode(size_t keybits)               \
 {                                                                              \
index cf9af3d..d796f36 100644 (file)
@@ -31,7 +31,7 @@ static void *camellia_dupctx(void *ctx)
         ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    *ret = *in;
+    in->base.hw->copyctx(&ret->base, &in->base);
 
     return ret;
 }
index 39ba4bd..fe844ae 100644 (file)
@@ -35,6 +35,8 @@ static int cipher_hw_camellia_initkey(PROV_CIPHER_CTX *dat,
     return 1;
 }
 
+IMPLEMENT_CIPHER_HW_COPYCTX(cipher_hw_camellia_copyctx, PROV_CAMELLIA_CTX)
+
 # if defined(SPARC_CMLL_CAPABLE)
 #  include "cipher_camellia_hw_t4.inc"
 # else
@@ -46,7 +48,8 @@ static int cipher_hw_camellia_initkey(PROV_CIPHER_CTX *dat,
 #define PROV_CIPHER_HW_camellia_mode(mode)                                     \
 static const PROV_CIPHER_HW camellia_##mode = {                                \
     cipher_hw_camellia_initkey,                                                \
-    cipher_hw_generic_##mode                                                   \
+    cipher_hw_generic_##mode,                                                  \
+    cipher_hw_camellia_copyctx                                                 \
 };                                                                             \
 PROV_CIPHER_HW_declare(mode)                                                   \
 const PROV_CIPHER_HW *PROV_CIPHER_HW_camellia_##mode(size_t keybits)           \
index 24e1046..96e5e20 100644 (file)
@@ -76,7 +76,8 @@ static int cipher_hw_camellia_t4_initkey(PROV_CIPHER_CTX *dat,
 #define PROV_CIPHER_HW_declare(mode)                                           \
 static const PROV_CIPHER_HW t4_camellia_##mode = {                             \
     cipher_hw_camellia_t4_initkey,                                             \
-    cipher_hw_generic_##mode                                                   \
+    cipher_hw_generic_##mode,                                                  \
+    cipher_hw_camellia_copyctx                                                 \
 };
 #define PROV_CIPHER_HW_select(mode)                                            \
 if (SPARC_CMLL_CAPABLE)                                                        \
index bf9d220..caa7ff9 100644 (file)
@@ -31,7 +31,7 @@ static void *sm4_dupctx(void *ctx)
         ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    *ret = *in;
+    in->base.hw->copyctx(&ret->base, &in->base);
 
     return ret;
 }
index 9ecaf0b..73e8828 100644 (file)
@@ -26,10 +26,13 @@ static int cipher_hw_sm4_initkey(PROV_CIPHER_CTX *ctx,
     return 1;
 }
 
+IMPLEMENT_CIPHER_HW_COPYCTX(cipher_hw_sm4_copyctx, PROV_SM4_CTX)
+
 # define PROV_CIPHER_HW_sm4_mode(mode)                                         \
 static const PROV_CIPHER_HW sm4_##mode = {                                     \
     cipher_hw_sm4_initkey,                                                     \
-    cipher_hw_chunked_##mode                                                   \
+    cipher_hw_chunked_##mode,                                                  \
+    cipher_hw_sm4_copyctx                                                      \
 };                                                                             \
 const PROV_CIPHER_HW *PROV_CIPHER_HW_sm4_##mode(size_t keybits)                \
 {                                                                              \
index 1039c21..43d6c48 100644 (file)
@@ -621,12 +621,15 @@ static int cipher_test_enc(EVP_TEST *t, int enc,
     unsigned char *in, *expected_out, *tmp = NULL;
     size_t in_len, out_len, donelen = 0;
     int ok = 0, tmplen, chunklen, tmpflen, i;
+    EVP_CIPHER_CTX *ctx_base = NULL;
     EVP_CIPHER_CTX *ctx = NULL;
 
     t->err = "TEST_FAILURE";
+    if (!TEST_ptr(ctx_base = EVP_CIPHER_CTX_new()))
+        goto err;
     if (!TEST_ptr(ctx = EVP_CIPHER_CTX_new()))
         goto err;
-    EVP_CIPHER_CTX_set_flags(ctx, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW);
+    EVP_CIPHER_CTX_set_flags(ctx_base, EVP_CIPHER_CTX_FLAG_WRAP_ALLOW);
     if (enc) {
         in = expected->plaintext;
         in_len = expected->plaintext_len;
@@ -663,18 +666,18 @@ static int cipher_test_enc(EVP_TEST *t, int enc,
         in = memcpy(tmp + out_misalign + in_len + 2 * EVP_MAX_BLOCK_LENGTH +
                     inp_misalign, in, in_len);
     }
-    if (!EVP_CipherInit_ex(ctx, expected->cipher, NULL, NULL, NULL, enc)) {
+    if (!EVP_CipherInit_ex(ctx_base, expected->cipher, NULL, NULL, NULL, enc)) {
         t->err = "CIPHERINIT_ERROR";
         goto err;
     }
     if (expected->iv) {
         if (expected->aead) {
-            if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_IVLEN,
+            if (!EVP_CIPHER_CTX_ctrl(ctx_base, EVP_CTRL_AEAD_SET_IVLEN,
                                      expected->iv_len, 0)) {
                 t->err = "INVALID_IV_LENGTH";
                 goto err;
             }
-        } else if (expected->iv_len != (size_t)EVP_CIPHER_CTX_iv_length(ctx)) {
+        } else if (expected->iv_len != (size_t)EVP_CIPHER_CTX_iv_length(ctx_base)) {
             t->err = "INVALID_IV_LENGTH";
             goto err;
         }
@@ -693,7 +696,7 @@ static int cipher_test_enc(EVP_TEST *t, int enc,
             tag = expected->tag;
         }
         if (tag || expected->aead != EVP_CIPH_GCM_MODE) {
-            if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG,
+            if (!EVP_CIPHER_CTX_ctrl(ctx_base, EVP_CTRL_AEAD_SET_TAG,
                                      expected->tag_len, tag))
                 goto err;
         }
@@ -702,25 +705,25 @@ static int cipher_test_enc(EVP_TEST *t, int enc,
     if (expected->rounds > 0) {
         int  rounds = (int)expected->rounds;
 
-        if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_SET_RC5_ROUNDS, rounds, NULL)) {
+        if (!EVP_CIPHER_CTX_ctrl(ctx_base, EVP_CTRL_SET_RC5_ROUNDS, rounds, NULL)) {
             t->err = "INVALID_ROUNDS";
             goto err;
         }
     }
 
-    if (!EVP_CIPHER_CTX_set_key_length(ctx, expected->key_len)) {
+    if (!EVP_CIPHER_CTX_set_key_length(ctx_base, expected->key_len)) {
         t->err = "INVALID_KEY_LENGTH";
         goto err;
     }
     if (expected->key_bits > 0) {
         int bits = (int)expected->key_bits;
 
-        if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_SET_RC2_KEY_BITS, bits, NULL)) {
+        if (!EVP_CIPHER_CTX_ctrl(ctx_base, EVP_CTRL_SET_RC2_KEY_BITS, bits, NULL)) {
             t->err = "INVALID KEY BITS";
             goto err;
         }
     }
-    if (!EVP_CipherInit_ex(ctx, NULL, NULL, expected->key, expected->iv, -1)) {
+    if (!EVP_CipherInit_ex(ctx_base, NULL, NULL, expected->key, expected->iv, -1)) {
         t->err = "KEY_SET_ERROR";
         goto err;
     }
@@ -729,11 +732,20 @@ static int cipher_test_enc(EVP_TEST *t, int enc,
     if (expected->iv != NULL
         && (EVP_CIPHER_flags(expected->cipher) & EVP_CIPH_CUSTOM_IV) == 0
         && !TEST_mem_eq(expected->iv, expected->iv_len,
-                        EVP_CIPHER_CTX_iv(ctx), expected->iv_len)) {
+                        EVP_CIPHER_CTX_iv(ctx_base), expected->iv_len)) {
         t->err = "INVALID_IV";
         goto err;
     }
 
+    /* Test that the cipher dup functions correctly if it is supported */
+    if (EVP_CIPHER_CTX_copy(ctx, ctx_base)) {
+        EVP_CIPHER_CTX_free(ctx_base);
+        ctx_base = NULL;
+    } else {
+        EVP_CIPHER_CTX_free(ctx);
+        ctx = ctx_base;
+    }
+
     if (expected->aead == EVP_CIPH_CCM_MODE) {
         if (!EVP_CipherUpdate(ctx, NULL, &tmplen, NULL, out_len)) {
             t->err = "CCM_PLAINTEXT_LENGTH_SET_ERROR";
@@ -840,6 +852,8 @@ static int cipher_test_enc(EVP_TEST *t, int enc,
     ok = 1;
  err:
     OPENSSL_free(tmp);
+    if (ctx != ctx_base)
+        EVP_CIPHER_CTX_free(ctx_base);
     EVP_CIPHER_CTX_free(ctx);
     return ok;
 }