From e8fb5d2c583c3fe49267fc476a2e8b827a388b6a Mon Sep 17 00:00:00 2001 From: jsing Date: Sat, 3 May 2014 16:54:48 +0000 Subject: [PATCH] Add checks for invalid base64 encoded data, specifically relating to the handling of padding. This fixes a crash that can be triggered by feeding base64 data followed by 64 or more padding characters, which results in a negative output length. This issue was reported by David Ramos, although the same bug has been sitting in the OpenSSL RT since 2011: https://rt.openssl.org/Ticket/Display.html?id=2608 Worse still, BIO_read seems to be completely unable to detect that the base64 input was invalid/corrupt - in particular, enabling BIO_FLAGS_BASE64_NO_NL results in a stream of zero value bytes rather than no input (possibly a good replacement for /dev/null...), which could result in nasty consequences. Prior to this fix some zero value bytes were also injected without this flag being enabled. The recently added base64 regress triggers and documents these issues (and also ensures that this change retains functional behaviour). --- lib/libcrypto/evp/encode.c | 12 ++++++++++++ lib/libssl/src/crypto/evp/encode.c | 12 ++++++++++++ 2 files changed, 24 insertions(+) diff --git a/lib/libcrypto/evp/encode.c b/lib/libcrypto/evp/encode.c index 9540a849c9f..2268b8d2f27 100644 --- a/lib/libcrypto/evp/encode.c +++ b/lib/libcrypto/evp/encode.c @@ -259,6 +259,12 @@ EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, goto end; } + /* There should not be base64 data after padding. */ + if (eof && tmp != '=' && tmp != '\r' && tmp != '\n') { + 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 @@ -269,6 +275,12 @@ EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, eof++; } + /* 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) diff --git a/lib/libssl/src/crypto/evp/encode.c b/lib/libssl/src/crypto/evp/encode.c index 9540a849c9f..2268b8d2f27 100644 --- a/lib/libssl/src/crypto/evp/encode.c +++ b/lib/libssl/src/crypto/evp/encode.c @@ -259,6 +259,12 @@ EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, goto end; } + /* There should not be base64 data after padding. */ + if (eof && tmp != '=' && tmp != '\r' && tmp != '\n') { + 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 @@ -269,6 +275,12 @@ EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, eof++; } + /* 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) -- 2.20.1