wbrawner/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

RT3757: base64 encoding bugs

Browse source 
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@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Dr Stephen Henson <steve@openssl.org>
This commit is contained in:
Emilia Kasper 2015-09-02 15:31:28 +02:00
parent 4bd16463b8
commit 3cdd1e94b1
4 changed files with 482 additions and 136 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
CHANGES
View file

@ -4,6 +4,12 @@
Changes between 1.0.2 and 1.1.0 [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]
*) New testing framework
The testing framework has been largely rewritten and is now using
perl and the perl modules Test::Harness and an extended variant of

186
crypto/evp/encode.c
View file

@ -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 a lot */
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;
goto end;
} else
ctx->length = v;
if (seof >= 0) {
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;
}
out += v;
OPENSSL_assert(n < (int)sizeof(ctx->enc_data));
d[n++] = tmp;
}
if (n == 64) {
decoded_len = EVP_DecodeBlock(out, d, n);
n = 0;
if (decoded_len < 0 || eof > decoded_len) {
rv = -1;
goto end;
}
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);
}

253
test/evp_test.c
View file

@ -124,12 +124,58 @@ static int parse_line(char **pkw, char **pval, char *linebuf)
return 1;
}
/*
* Unescape some escape sequences in string literals.
* Return the result in a newly allocated buffer.
* Currently only supports '\n'.
* If the input length is 0, returns a valid 1-byte buffer, but sets
* the length to 0.
*/
static unsigned char* unescape(const char *input, size_t input_len,
size_t *out_len)
{
unsigned char *ret, *p;
size_t i;
if (input_len == 0) {
*out_len = 0;
return OPENSSL_zalloc(1);
}
/* Escaping is non-expanding; over-allocate original size for simplicity. */
ret = p = OPENSSL_malloc(input_len);
if (ret == NULL)
return NULL;
for (i = 0; i < input_len; i++) {
if (input[i] == '\\') {
if (i == input_len - 1 || input[i+1] != 'n')
goto err;
*p++ = '\n';
i++;
} else {
*p++ = input[i];
}
}
*out_len = p - ret;
return ret;
err:
OPENSSL_free(ret);
return NULL;
}
/* For a hex string "value" convert to a binary allocated buffer */
static int test_bin(const char *value, unsigned char **buf, size_t *buflen)
{
long len;
if (!*value) {
/* Don't return NULL for zero length buffer */
/*
* Don't return NULL for zero length buffer.
* This is needed for some tests with empty keys: HMAC_Init_ex() expects
* a non-NULL key buffer even if the key length is 0, in order to detect
* key reset.
*/
*buf = OPENSSL_malloc(1);
if (!*buf)
return 0;
@ -145,19 +191,12 @@ static int test_bin(const char *value, unsigned char **buf, size_t *buflen)
if (value[vlen - 1] != '"')
return 0;
vlen--;
if (vlen == 0) {
*buf = OPENSSL_malloc(1);
if (*buf == NULL)
return 0;
**buf = 0;
} else {
*buf = BUF_memdup(value, vlen);
if (*buf == NULL)
return 0;
}
*buflen = vlen;
*buf = unescape(value, vlen, buflen);
if (*buf == NULL)
return 0;
return 1;
}
*buf = string_to_hex(value, &len);
if (!*buf) {
fprintf(stderr, "Value=%s\n", value);
@ -217,9 +256,10 @@ struct evp_test {
/* Number of tests skipped */
int nskip;
/* If output mismatch expected and got value */
unsigned char *out_got;
unsigned char *out_received;
size_t out_received_len;
unsigned char *out_expected;
size_t out_len;
size_t out_expected_len;
/* test specific data */
void *data;
/* Current test should be skipped */
@ -252,6 +292,7 @@ static const struct evp_test_method psign_test_method, pverify_test_method;
static const struct evp_test_method pdecrypt_test_method;
static const struct evp_test_method pverify_recover_test_method;
static const struct evp_test_method pbe_test_method;
static const struct evp_test_method encode_test_method;
static const struct evp_test_method *evp_test_list[] = {
&digest_test_method,
@ -262,6 +303,7 @@ static const struct evp_test_method *evp_test_list[] = {
&pdecrypt_test_method,
&pverify_recover_test_method,
&pbe_test_method,
&encode_test_method,
NULL
};
@ -290,17 +332,21 @@ static void free_expected(struct evp_test *t)
OPENSSL_free(t->expected_err);
t->expected_err = NULL;
OPENSSL_free(t->out_expected);
OPENSSL_free(t->out_got);
OPENSSL_free(t->out_received);
t->out_expected = NULL;
t->out_got = NULL;
t->out_received = NULL;
t->out_expected_len = 0;
t->out_received_len = 0;
/* Literals. */
t->err = NULL;
}
static void print_expected(struct evp_test *t)
{
if (t->out_expected == NULL)
if (t->out_expected == NULL && t->out_received == NULL)
return;
hex_print("Expected:", t->out_expected, t->out_len);
hex_print("Got: ", t->out_got, t->out_len);
hex_print("Expected:", t->out_expected, t->out_expected_len);
hex_print("Got: ", t->out_received, t->out_received_len);
free_expected(t);
}
@ -496,21 +542,37 @@ static int process_test(struct evp_test *t, char *buf, int verbose)
return 1;
}
static int check_output(struct evp_test *t, const unsigned char *expected,
const unsigned char *got, size_t len)
static int check_var_length_output(struct evp_test *t,
const unsigned char *expected,
size_t expected_len,
const unsigned char *received,
size_t received_len)
{
if (!memcmp(expected, got, len))
if (expected_len == received_len &&
memcmp(expected, received, expected_len) == 0) {
return 0;
t->out_expected = BUF_memdup(expected, len);
t->out_got = BUF_memdup(got, len);
t->out_len = len;
if (t->out_expected == NULL || t->out_got == NULL) {
}
/* The result printing code expects a non-NULL buffer. */
t->out_expected = BUF_memdup(expected, expected_len ? expected_len : 1);
t->out_expected_len = expected_len;
t->out_received = BUF_memdup(received, received_len ? received_len : 1);
t->out_received_len = received_len;
if (t->out_expected == NULL || t->out_received == NULL) {
fprintf(stderr, "Memory allocation error!\n");
exit(1);
}
return 1;
}
static int check_output(struct evp_test *t,
const unsigned char *expected,
const unsigned char *received,
size_t len)
{
return check_var_length_output(t, expected, len, received, len);
}
int main(int argc, char **argv)
{
FILE *in = NULL;
@ -528,17 +590,7 @@ int main(int argc, char **argv)
OpenSSL_add_all_algorithms();
memset(&t, 0, sizeof(t));
t.meth = NULL;
t.public = NULL;
t.private = NULL;
t.err = NULL;
t.line = 0;
t.start_line = -1;
t.errors = 0;
t.ntests = 0;
t.out_expected = NULL;
t.out_got = NULL;
t.out_len = 0;
in = fopen(argv[1], "r");
t.in = in;
while (fgets(buf, sizeof(buf), in)) {
@ -1473,3 +1525,132 @@ static const struct evp_test_method pbe_test_method = {
pbe_test_parse,
pbe_test_run
};
/* Base64 tests */
typedef enum {
BASE64_CANONICAL_ENCODING = 0,
BASE64_VALID_ENCODING = 1,
BASE64_INVALID_ENCODING = 2
} base64_encoding_type;
struct encode_data {
/* Input to encoding */
unsigned char *input;
size_t input_len;
/* Expected output */
unsigned char *output;
size_t output_len;
base64_encoding_type encoding;
};
static int encode_test_init(struct evp_test *t, const char *encoding)
{
struct encode_data *edata = OPENSSL_zalloc(sizeof(*edata));
if (strcmp(encoding, "canonical") == 0) {
edata->encoding = BASE64_CANONICAL_ENCODING;
} else if (strcmp(encoding, "valid") == 0) {
edata->encoding = BASE64_VALID_ENCODING;
} else if (strcmp(encoding, "invalid") == 0) {
edata->encoding = BASE64_INVALID_ENCODING;
t->expected_err = BUF_strdup("DECODE_ERROR");
if (t->expected_err == NULL)
return 0;
} else {
fprintf(stderr, "Bad encoding: %s. Should be one of "
"{canonical, valid, invalid}\n", encoding);
return 0;
}
t->data = edata;
return 1;
}
static void encode_test_cleanup(struct evp_test *t)
{
struct encode_data *edata = t->data;
test_free(edata->input);
test_free(edata->output);
memset(edata, 0, sizeof(*edata));
}
static int encode_test_parse(struct evp_test *t,
const char *keyword, const char *value)
{
struct encode_data *edata = t->data;
if (strcmp(keyword, "Input") == 0)
return test_bin(value, &edata->input, &edata->input_len);
if (strcmp(keyword, "Output") == 0)
return test_bin(value, &edata->output, &edata->output_len);
return 0;
}
static int encode_test_run(struct evp_test *t)
{
struct encode_data *edata = t->data;
unsigned char *encode_out = NULL, *decode_out = NULL;
int output_len, chunk_len;
const char *err = "INTERNAL_ERROR";
EVP_ENCODE_CTX decode_ctx;
if (edata->encoding == BASE64_CANONICAL_ENCODING) {
EVP_ENCODE_CTX encode_ctx;
encode_out = OPENSSL_malloc(EVP_ENCODE_LENGTH(edata->input_len));
if (encode_out == NULL)
goto err;
EVP_EncodeInit(&encode_ctx);
EVP_EncodeUpdate(&encode_ctx, encode_out, &chunk_len,
edata->input, edata->input_len);
output_len = chunk_len;
EVP_EncodeFinal(&encode_ctx, encode_out + chunk_len, &chunk_len);
output_len += chunk_len;
if (check_var_length_output(t, edata->output, edata->output_len,
encode_out, output_len)) {
err = "BAD_ENCODING";
goto err;
}
}
decode_out = OPENSSL_malloc(EVP_DECODE_LENGTH(edata->output_len));
if (decode_out == NULL)
goto err;
EVP_DecodeInit(&decode_ctx);
if (EVP_DecodeUpdate(&decode_ctx, decode_out, &chunk_len, edata->output,
edata->output_len) < 0) {
err = "DECODE_ERROR";
goto err;
}
output_len = chunk_len;
if (EVP_DecodeFinal(&decode_ctx, decode_out + chunk_len, &chunk_len) != 1) {
err = "DECODE_ERROR";
goto err;
}
output_len += chunk_len;
if (edata->encoding != BASE64_INVALID_ENCODING &&
check_var_length_output(t, edata->input, edata->input_len,
decode_out, output_len)) {
err = "BAD_DECODING";
goto err;
}
err = NULL;
err:
t->err = err;
OPENSSL_free(encode_out);
OPENSSL_free(decode_out);
return 1;
}
static const struct evp_test_method encode_test_method = {
"Encoding",
encode_test_init,
encode_test_cleanup,
encode_test_parse,
encode_test_run,
};

173
test/evptests.txt
View file

@ -2624,3 +2624,176 @@ Salt = 7361006c74
iter = 4096
MD = sha512
Key = 9d9e9c4cd21fe4be24d5b8244c759665
# Base64 tests
Encoding = canonical
Input = ""
Output = ""
Encoding = canonical
Input = "h"
Output = "aA==\n"
Encoding = canonical
Input = "hello"
Output = "aGVsbG8=\n"
Encoding = canonical
Input = "hello world!"
Output = "aGVsbG8gd29ybGQh\n"
Encoding = canonical
Input = 00010203040506070809a0b0c0d0e0f000
Output = "AAECAwQFBgcICaCwwNDg8AA=\n"
# Missing padding
Encoding = invalid
Output = "aGVsbG8"
Encoding = invalid
Output = "aGVsbG8\n"
# Tolerate missing newline
Encoding = valid
Input = "hello"
Output = "aGVsbG8="
# Don't tolerate extra trailing '='
Encoding = invalid
Input = "hello"
Output = "aGVsbG8==\n"
Encoding = invalid
Output = "aGVsbG8===\n"
# Don't tolerate data after '='
Encoding = invalid
Output = "aGV=sbG8=\n"
# Newlines are ignored
Encoding = valid
Input = "hello"
Output = "aGV\nsbG8=\n"
Encoding = canonical
Input = "hello"
Output = 614756736247383d0a
# Invalid characters
Encoding = invalid
Output = 614756736247383d0a00
Encoding = invalid
Output = 61475600736247383d0a
Encoding = invalid
Output = 61475601736247383d0a
Encoding = canonical
Input = "OpenSSLOpenSSL\n"
Output = "T3BlblNTTE9wZW5TU0wK\n"
Encoding = valid
Input = "OpenSSLOpenSSL\n"
Output = "T3BlblNTTE9wZW5TU0wK"
# Truncate 1-3 chars
Encoding = invalid
Output = "T3BlblNTTE9wZW5TU0w"
Encoding = invalid
Output = "T3BlblNTTE9wZW5TU0"
Encoding = invalid
Output = "T3BlblNTTE9wZW5TU"
Encoding = invalid
Output = "T3BlblNTTE9wZW5TU0wK===="
Encoding = invalid
Output = "T3BlblNTTE9wZW5TU0wK============================================\n"
Encoding = invalid
Output = "YQ==YQ==YQ==\n"
Encoding = invalid
Output = "A"
Encoding = invalid
Output = "A\n"
Encoding = invalid
Output = "A="
Encoding = invalid
Output = "A==\n"
Encoding = invalid
Output = "A===\n"
Encoding = invalid
Output = "A====\n"
Encoding = valid
Input = "OpenSSLOpenSSL\n"
Output = "T3BlblNTTE9wZW5TU0wK\n\n"
Encoding = valid
Input = "OpenSSLOpenSSL\n"
Output = "T3BlblNTTE\n9wZW5TU0wK"
# CVE 2015-0292
Encoding = invalid
Output = "ZW5jb2RlIG1lCg==================================================================\n"
Encoding = canonical
Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA==\n"
Encoding = valid
Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA\n==\n"
Encoding = valid
Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA=\n=\n"
Encoding = invalid
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA====\n"
# Multiline output without padding
Encoding = canonical
Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4\neHh4eHh4eHh4eHh4\n"
# Multiline output with padding
Encoding = canonical
Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4\neHh4eHh4eHh4eHh4eHh4eA==\n"
# Multiline output with line break in the middle of a b64 block is accepted
Encoding = valid
Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh\n4eHh4eHh4eHh4eHh4eHh4eA==\n"
# Long lines are accepted
Encoding = valid
Input = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA==\n"
# Multiline input with data after '='.
Encoding = invalid
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eA==\neHh4eHh4eHh4eHh4eHh4eHh4\n"
Encoding = invalid
Output = "eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4\neA==eHh4eHh4eHh4eHh4eHh4\n"
# B64_EOF ('-') terminates input and trailing bytes are ignored
Encoding = valid
Input = "OpenSSLOpenSSL\n"
Output = "T3BlblNTTE9wZW5TU0wK\n-abcd"
Encoding = valid
Input = "OpenSSLOpenSSL\n"
Output = "T3BlblNTTE9wZW5TU0wK-abcd"