Browse Source

Avoid theoretical overflows in EVP_EncodeUpdate.

See also upstream's 172c6e1e14defe7d49d62f5fc9ea6a79b225424f, but note our
values have different types. In particular, because we put in_len in a size_t
and C implicitly requires that all valid buffers' lengths fit in a ptrdiff_t
(signed), the overflow was impossible, assuming EVP_ENCODE_CTX::length is
untouched externally.

More importantly, this function is stuck taking an int output and has no return
value, so the only plausible contract is the caller is responsible for ensuring
the length fits anyway. Indeed, callers all call EVP_EncodeUpdate in bounded
chunks, so upstream's analysis is off.

Anyway, in theory that logic could locally overflow, so tweak it slightly. Tidy
up some of the variable names while I'm here.

Change-Id: Ifa78707cc26c11e0d67019918a028531b3d6738c
Reviewed-on: https://boringssl-review.googlesource.com/7847
Reviewed-by: Adam Langley <agl@google.com>
kris/onging/CECPQ3_patch15
David Benjamin 8 years ago
committed by Adam Langley
parent
commit
de2cf273d7
1 changed files with 16 additions and 13 deletions
  1. +16
    -13
      crypto/base64/base64.c

+ 16
- 13
crypto/base64/base64.c View File

@@ -119,8 +119,7 @@ void EVP_EncodeInit(EVP_ENCODE_CTX *ctx) {

void EVP_EncodeUpdate(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len,
const uint8_t *in, size_t in_len) {
unsigned i, j;
unsigned total = 0;
size_t total = 0;

*out_len = 0;
if (in_len == 0) {
@@ -128,33 +127,37 @@ void EVP_EncodeUpdate(EVP_ENCODE_CTX *ctx, uint8_t *out, int *out_len,
}

assert(ctx->length <= sizeof(ctx->enc_data));
assert(ctx->num < ctx->length);

if (ctx->num + in_len < ctx->length) {
if (ctx->length - ctx->num > in_len) {
memcpy(&ctx->enc_data[ctx->num], in, in_len);
ctx->num += in_len;
return;
}

if (ctx->num != 0) {
i = ctx->length - ctx->num;
memcpy(&ctx->enc_data[ctx->num], in, i);
in += i;
in_len -= i;
j = EVP_EncodeBlock(out, ctx->enc_data, ctx->length);
size_t todo = ctx->length - ctx->num;
memcpy(&ctx->enc_data[ctx->num], in, todo);
in += todo;
in_len -= todo;
size_t encoded = EVP_EncodeBlock(out, ctx->enc_data, ctx->length);
ctx->num = 0;
out += j;
out += encoded;
*(out++) = '\n';
*out = '\0';
total = j + 1;
total = encoded + 1;
}

while (in_len >= ctx->length) {
j = EVP_EncodeBlock(out, in, ctx->length);
size_t encoded = EVP_EncodeBlock(out, in, ctx->length);
in += ctx->length;
in_len -= ctx->length;
out += j;
out += encoded;
*(out++) = '\n';
*out = '\0';
total += j + 1;
total += encoded + 1;
}

if (in_len != 0) {
memcpy(&ctx->enc_data[0], in, in_len);
}


Loading…
Cancel
Save