Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenSSL 1.0.2 branch on uninitialized memory in EVP_CIPHER_CTX_copy #8980

Closed
guidovranken opened this issue May 22, 2019 · 2 comments
Closed

Comments

@guidovranken
Copy link
Contributor

#include <openssl/evp.h>

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }
#define CF_CHECK_NE(expr, res) if ( (expr) == (res) ) { goto end; }

int main(void)
{
    const EVP_CIPHER* cipher = NULL;
    EVP_CIPHER_CTX* ctx = EVP_CIPHER_CTX_new(), *ctx2 = NULL;
    CF_CHECK_NE(cipher = EVP_aes_128_gcm(), NULL);
    CF_CHECK_EQ(EVP_EncryptInit_ex(ctx, cipher, NULL, NULL, NULL), 1);
    ctx2 = EVP_CIPHER_CTX_new();
    EVP_CIPHER_CTX_copy(ctx2, ctx);
end:
    return 0;
}

Branching on an uninitialized variable occurs at line 1261 here

1257     case EVP_CTRL_COPY:
1258         {
1259             EVP_CIPHER_CTX *out = ptr;
1260             EVP_AES_GCM_CTX *gctx_out = out->cipher_data;
1261             if (gctx->gcm.key) {
1262                 if (gctx->gcm.key != &gctx->ks)
1263                     return 0;
1264                 gctx_out->gcm.key = &gctx_out->ks;
1265             }
1266             if (gctx->iv == c->iv)
1267                 gctx_out->iv = out->iv;
1268             else {
1269                 gctx_out->iv = OPENSSL_malloc(gctx->ivlen);
1270                 if (!gctx_out->iv)
1271                     return 0;
1272                 memcpy(gctx_out->iv, gctx->iv, gctx->ivlen);
1273             }
1274             return 1;
1275         }

You could argue my PoC is invalid API use. This doesn't seem to happen in the master branch, though. If you decide not to fix this, I will work around it in my fuzzer.

@mattcaswell
Copy link
Member

Hmm. Probably this is just a bug. Since 1.0.2 is in security-fix only mode I think we'll probably not fix this.

@guidovranken
Copy link
Contributor Author

OK I will work around it in my fuzzer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants