kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Maintain EVP_MD_CTX invariants.

Browse Source 
Thanks to Lennart Beringer for pointing that that malloc failures could
lead to invalid EVP_MD_CTX states. This change cleans up the code in
general so that fallible operations are all performed before mutating
objects. Thus failures should leave objects in a valid state.

Also, |ctx_size| is never zero and a hash with no context is not
sensible, so stop handling that case and simply assert that it doesn't
occur.

Change-Id: Ia60c3796dcf2f772f55e12e49431af6475f64d52
Reviewed-on: https://boringssl-review.googlesource.com/20544
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
This commit is contained in:
Adam Langley 2017-09-19 09:53:19 -07:00
parent 40b24c8154
commit 6b35262272
1 changed files with 37 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
crypto/fipsmodule/digest/digest.c
View File

@ -90,9 +90,7 @@ EVP_MD_CTX *EVP_MD_CTX_create(void) {
}
int EVP_MD_CTX_cleanup(EVP_MD_CTX *ctx) {
if (ctx->digest && ctx->digest->ctx_size && ctx->md_data) {
OPENSSL_free(ctx->md_data);
}
OPENSSL_free(ctx->md_data);
assert(ctx->pctx == NULL || ctx->pctx_ops != NULL);
if (ctx->pctx_ops) {
@ -114,17 +112,35 @@ void EVP_MD_CTX_destroy(EVP_MD_CTX *ctx) {
}
int EVP_MD_CTX_copy_ex(EVP_MD_CTX *out, const EVP_MD_CTX *in) {
uint8_t *tmp_buf = NULL;
if (in == NULL || in->digest == NULL) {
OPENSSL_PUT_ERROR(DIGEST, DIGEST_R_INPUT_NOT_INITIALIZED);
return 0;
}
if (out->digest == in->digest) {
// |md_data| will be the correct size in this case so it's removed from
// |out| at this point so that |EVP_MD_CTX_cleanup| doesn't free it and
// then it's reused.
EVP_PKEY_CTX *pctx = NULL;
assert(in->pctx == NULL || in->pctx_ops != NULL);
if (in->pctx) {
pctx = in->pctx_ops->dup(in->pctx);
if (!pctx) {
OPENSSL_PUT_ERROR(DIGEST, ERR_R_MALLOC_FAILURE);
return 0;
}
}
uint8_t *tmp_buf;
if (out->digest != in->digest) {
assert(in->digest->ctx_size != 0);
tmp_buf = OPENSSL_malloc(in->digest->ctx_size);
if (tmp_buf == NULL) {
if (pctx) {
in->pctx_ops->free(pctx);
}
OPENSSL_PUT_ERROR(DIGEST, ERR_R_MALLOC_FAILURE);
return 0;
}
} else {
// |md_data| will be the correct size in this case. It's removed from |out|
// so that |EVP_MD_CTX_cleanup| doesn't free it, and then it's reused.
tmp_buf = out->md_data;
out->md_data = NULL;
}
@ -132,28 +148,11 @@ int EVP_MD_CTX_copy_ex(EVP_MD_CTX *out, const EVP_MD_CTX *in) {
EVP_MD_CTX_cleanup(out);
out->digest = in->digest;
if (in->md_data && in->digest->ctx_size) {
if (tmp_buf) {
out->md_data = tmp_buf;
} else {
out->md_data = OPENSSL_malloc(in->digest->ctx_size);
if (!out->md_data) {
OPENSSL_PUT_ERROR(DIGEST, ERR_R_MALLOC_FAILURE);
return 0;
}
}
OPENSSL_memcpy(out->md_data, in->md_data, in->digest->ctx_size);
}
assert(in->pctx == NULL || in->pctx_ops != NULL);
out->md_data = tmp_buf;
OPENSSL_memcpy(out->md_data, in->md_data, in->digest->ctx_size);
out->pctx = pctx;
out->pctx_ops = in->pctx_ops;
if (in->pctx && in->pctx_ops) {
out->pctx = in->pctx_ops->dup(in->pctx);
if (!out->pctx) {
EVP_MD_CTX_cleanup(out);
return 0;
}
}
assert(out->pctx == NULL || out->pctx_ops != NULL);
return 1;
}
@ -165,18 +164,16 @@ int EVP_MD_CTX_copy(EVP_MD_CTX *out, const EVP_MD_CTX *in) {
int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *engine) {
if (ctx->digest != type) {
if (ctx->digest && ctx->digest->ctx_size > 0) {
OPENSSL_free(ctx->md_data);
ctx->md_data = NULL;
assert(type->ctx_size != 0);
uint8_t *md_data = OPENSSL_malloc(type->ctx_size);
if (md_data == NULL) {
OPENSSL_PUT_ERROR(DIGEST, ERR_R_MALLOC_FAILURE);
return 0;
}
OPENSSL_free(ctx->md_data);
ctx->md_data = md_data;
ctx->digest = type;
if (type->ctx_size > 0) {
ctx->md_data = OPENSSL_malloc(type->ctx_size);
if (ctx->md_data == NULL) {
OPENSSL_PUT_ERROR(DIGEST, ERR_R_MALLOC_FAILURE);
return 0;
}
}
}
assert(ctx->pctx == NULL || ctx->pctx_ops != NULL);