[openssl-commits] [openssl] OpenSSL_1_0_1-stable update
Emilia Kasper
emilia at openssl.org
Thu Sep 17 18:28:17 UTC 2015
The branch OpenSSL_1_0_1-stable has been updated
via 76067c75fd60371c0a66a36ed531e52b883dcf6a (commit)
from f95d1af064bd0477cb551124bb3d7792c4e3216b (commit)
- Log -----------------------------------------------------------------
commit 76067c75fd60371c0a66a36ed531e52b883dcf6a
Author: Emilia Kasper <emilia at openssl.org>
Date: Wed Sep 2 15:31:28 2015 +0200
RT3757: base64 encoding bugs
Rewrite EVP_DecodeUpdate.
In particular: reject extra trailing padding, and padding in the middle
of the content. Don't limit line length. Add tests.
Previously, the behaviour was ill-defined, and depended on the position
of the padding within the input.
In addition, this appears to fix a possible two-byte oob read.
Reviewed-by: Richard Levitte <levitte at openssl.org>
Reviewed-by: Rich Salz <rsalz at openssl.org>
Reviewed-by: Dr Stephen Henson <steve at openssl.org>
(cherry picked from commit 3cdd1e94b1d71f2ce3002738f9506da91fe2af45)
(cherry picked from commit 37faf117965de181f4de0b4032eecac2566de5f6)
-----------------------------------------------------------------------
Summary of changes:
CHANGES | 6 ++
crypto/evp/encode.c | 182 ++++++++++++++++++++++++----------------------------
2 files changed, 90 insertions(+), 98 deletions(-)
diff --git a/CHANGES b/CHANGES
index 3ac66ae..178d010 100644
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,12 @@
Changes between 1.0.1p and 1.0.1q [xx XXX xxxx]
+ *) Rewrite EVP_DecodeUpdate (base64 decoding) to fix several bugs.
+ This changes the decoding behaviour for some invalid messages,
+ though the change is mostly in the more lenient direction, and
+ legacy behaviour is preserved as much as possible.
+ [Emilia Käsper]
+
*) In DSA_generate_parameters_ex, if the provided seed is too short,
return an error
[Rich Salz and Ismo Puustinen <ismo.puustinen at intel.com>]
diff --git a/crypto/evp/encode.c b/crypto/evp/encode.c
index 5c5988f..f758a8c 100644
--- a/crypto/evp/encode.c
+++ b/crypto/evp/encode.c
@@ -103,6 +103,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,
@@ -218,8 +219,9 @@ int EVP_EncodeBlock(unsigned char *t, const unsigned char *f, int dlen)
void EVP_DecodeInit(EVP_ENCODE_CTX *ctx)
{
- ctx->length = 30;
+ /* Only ctx->num is used during decoding. */
ctx->num = 0;
+ ctx->length = 0;
ctx->line_num = 0;
ctx->expect_nl = 0;
}
@@ -228,139 +230,123 @@ void EVP_DecodeInit(EVP_ENCODE_CTX *ctx)
* -1 for error
* 0 for last line
* 1 for full line
+ *
+ * Note: even though EVP_DecodeUpdate attempts to detect and report end of
+ * content, the context doesn't currently remember it and will accept more data
+ * in the next call. Therefore, the caller is responsible for checking and
+ * rejecting a 0 return value in the middle of content.
+ *
+ * Note: even though EVP_DecodeUpdate has historically tried to detect end of
+ * content based on line length, this has never worked properly. Therefore,
+ * we now return 0 when one of the following is true:
+ * - Padding or B64_EOF was detected and the last block is complete.
+ * - Input has zero-length.
+ * -1 is returned if:
+ * - Invalid characters are detected.
+ * - There is extra trailing padding, or data after padding.
+ * - B64_EOF is detected after an incomplete base64 block.
*/
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++);
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) {
+ if (v == B64_ERROR) {
rv = -1;
goto end;
}
- /*
- * have we seen a '=' which is 'definitly' 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;
}
- if (v == B64_CR) {
- ln = 0;
- if (exp_nl)
- continue;
+ if (eof > 2) {
+ rv = -1;
+ goto end;
}
- /* eoln */
- if (v == B64_EOLN) {
- ln = 0;
- if (exp_nl) {
- exp_nl = 0;
- continue;
- }
- }
- 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 '=' */
+ if (v == B64_EOF) {
+ seof = 1;
+ goto tail;
}
- 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;
- }
- if (eof > v) {
- rv = -1;
- 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;
+ /* 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;
- } else
- ctx->length = v;
+ }
+ OPENSSL_assert(n < (int)sizeof(ctx->enc_data));
+ d[n++] = tmp;
+ }
- if (seof >= 0) {
- 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;
}
- out += v;
+ ret += decoded_len - eof;
+ out += decoded_len - eof;
}
}
- rv = 1;
- end:
+
+ /*
+ * 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;
+ }
+ ret += (decoded_len - eof);
+ } else if (seof) {
+ /* EOF in the middle of a base64 block. */
+ rv = -1;
+ goto 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);
}
More information about the openssl-commits
mailing list