From dad60cb9f024c77a6bb02993f7d77da011b245a1 Mon Sep 17 00:00:00 2001 From: inoguchi <> Date: Tue, 3 Mar 2020 15:03:14 +0000 Subject: Fix base64 processing of long lines Fix the problem that long unbroken line of base64 text is not decoded. Referred to this OpenSSL commit and adapted to the codebase. 3cdd1e94b1d71f2ce3002738f9506da91fe2af45 Reported by john.a.passaro gmail.com to the LibreSSL ML. ok tb@ --- src/lib/libcrypto/evp/encode.c | 165 +++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 99 deletions(-) diff --git a/src/lib/libcrypto/evp/encode.c b/src/lib/libcrypto/evp/encode.c index ae107abb87..95dc79d70d 100644 --- a/src/lib/libcrypto/evp/encode.c +++ b/src/lib/libcrypto/evp/encode.c @@ -1,4 +1,4 @@ -/* $OpenBSD: encode.c,v 1.26 2019/01/19 01:24:18 tb Exp $ */ +/* $OpenBSD: encode.c,v 1.27 2020/03/03 15:03:14 inoguchi Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -92,6 +92,7 @@ abcdefghijklmnopqrstuvwxyz0123456789+/"; #define B64_WS 0xE0 #define B64_ERROR 0xFF #define B64_NOT_BASE64(a) (((a)|0x13) == 0xF3) +#define B64_BASE64(a) !B64_NOT_BASE64(a) static const unsigned char data_ascii2bin[128] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, @@ -231,151 +232,117 @@ EVP_EncodeBlock(unsigned char *t, const unsigned char *f, int dlen) void EVP_DecodeInit(EVP_ENCODE_CTX *ctx) { - ctx->length = 30; ctx->num = 0; + ctx->length = 0; ctx->line_num = 0; ctx->expect_nl = 0; } -/* -1 for error - * 0 for last line - * 1 for full line - */ int EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, const unsigned char *in, int inl) { - int seof = -1, eof = 0, rv = -1, ret = 0, i, v, tmp, n, ln, exp_nl; + int seof = 0, eof = 0, rv = -1, ret = 0, i, v, tmp, n, decoded_len; unsigned char *d; n = ctx->num; d = ctx->enc_data; - ln = ctx->line_num; - exp_nl = ctx->expect_nl; - /* last line of input. */ - if ((inl == 0) || ((n == 0) && (conv_ascii2bin(in[0]) == B64_EOF))) { + if (n > 0 && d[n - 1] == '=') { + eof++; + if (n > 1 && d[n - 2] == '=') + eof++; + } + + /* Legacy behaviour: an empty input chunk signals end of input. */ + if (inl == 0) { rv = 0; goto end; } - /* We parse the input data */ for (i = 0; i < inl; i++) { - /* If the current line is > 80 characters, scream alot */ - if (ln >= 80) { - rv = -1; - goto end; - } - - /* Get char and put it into the buffer */ - tmp= *(in++); + tmp = *(in++); v = conv_ascii2bin(tmp); - /* only save the good data :-) */ - if (!B64_NOT_BASE64(v)) { - OPENSSL_assert(n < (int)sizeof(ctx->enc_data)); - d[n++] = tmp; - ln++; - } else if (v == B64_ERROR) { - rv = -1; - goto end; - } - - /* There should not be base64 data after padding. */ - if (eof && tmp != '=' && tmp != '\r' && tmp != '\n' && - v != B64_EOF) { + if (v == B64_ERROR) { rv = -1; goto end; } - /* have we seen a '=' which is 'definitely' the last - * input line. seof will point to the character that - * holds it. and eof will hold how many characters to - * chop off. */ if (tmp == '=') { - if (seof == -1) - seof = n; eof++; + } else if (eof > 0 && B64_BASE64(v)) { + /* More data after padding. */ + rv = -1; + goto end; } - /* There should be no more than two padding markers. */ if (eof > 2) { rv = -1; goto end; } - if (v == B64_CR) { - ln = 0; - if (exp_nl) - continue; + if (v == B64_EOF) { + seof = 1; + goto tail; } - /* eoln */ - if (v == B64_EOLN) { - ln = 0; - if (exp_nl) { - exp_nl = 0; - continue; + /* Only save valid base64 characters. */ + if (B64_BASE64(v)) { + if (n >= 64) { + /* + * We increment n once per loop, and empty the + * buffer as soon as we reach 64 characters, so + * this can only happen if someone's manually + * messed with the ctx. Refuse to write any + * more data. + */ + rv = -1; + goto end; } - } - exp_nl = 0; - - /* If we are at the end of input and it looks like a - * line, process it. */ - if (((i + 1) == inl) && (((n&3) == 0) || eof)) { - v = B64_EOF; - /* In case things were given us in really small - records (so two '=' were given in separate - updates), eof may contain the incorrect number - of ending bytes to skip, so let's redo the count */ - eof = 0; - if (d[n-1] == '=') - eof++; - if (d[n-2] == '=') - eof++; - /* There will never be more than two '=' */ + OPENSSL_assert(n < (int)sizeof(ctx->enc_data)); + d[n++] = tmp; } - if ((v == B64_EOF && (n&3) == 0) || (n >= 64)) { - /* This is needed to work correctly on 64 byte input - * lines. We process the line and then need to - * accept the '\n' */ - if ((v != B64_EOF) && (n >= 64)) - exp_nl = 1; - if (n > 0) { - v = EVP_DecodeBlock(out, d, n); - n = 0; - if (v < 0) { - rv = 0; - goto end; - } - ret += (v - eof); - } else { - eof = 1; - v = 0; - } - - /* This is the case where we have had a short - * but valid input line */ - if ((v < ctx->length) && eof) { - rv = 0; + if (n == 64) { + decoded_len = EVP_DecodeBlock(out, d, n); + n = 0; + if (decoded_len < 0 || eof > decoded_len) { + rv = -1; goto end; - } else - ctx->length = v; + } + ret += decoded_len - eof; + out += decoded_len - eof; + } + } - if (seof >= 0) { - rv = 0; + /* + * Legacy behaviour: if the current line is a full base64-block (i.e., + * has 0 mod 4 base64 characters), it is processed immediately. We keep + * this behaviour as applications may not be calling EVP_DecodeFinal + * properly. + */ + tail: + if (n > 0) { + if ((n & 3) == 0) { + decoded_len = EVP_DecodeBlock(out, d, n); + n = 0; + if (decoded_len < 0 || eof > decoded_len) { + rv = -1; goto end; } - out += v; + ret += (decoded_len - eof); + } else if (seof) { + /* EOF in the middle of a base64 block. */ + rv = -1; + goto end; } } - rv = 1; -end: + rv = seof || (n == 0 && eof) ? 0 : 1; + end: + /* Legacy behaviour. This should probably rather be zeroed on error. */ *outl = ret; ctx->num = n; - ctx->line_num = ln; - ctx->expect_nl = exp_nl; return (rv); } -- cgit v1.2.3-55-g6feb