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>
diff --git a/crypto/base64/base64.c b/crypto/base64/base64.c
index 4822fb8..61f79cd 100644
--- a/crypto/base64/base64.c
+++ b/crypto/base64/base64.c
@@ -119,8 +119,7 @@
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 @@
}
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);
}
