diff --git a/AUTHORS.md b/AUTHORS.md index f9a2903050..5b036a0d83 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -49,4 +49,5 @@ Individuals * Tim Hudson * Tomáš Mráz * Ulf Möller + * Valerii Krygin * Viktor Dukhovni diff --git a/CHANGES.md b/CHANGES.md index 67d97e9e6d..d846e789a8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -232,6 +232,22 @@ OpenSSL 3.5 *Pablo De Lara Guarch, Dan Pittman* + * Fix EVP_DecodeUpdate(): do not write padding zeros to the decoded output. + + According to the documentation, + for every 4 valid base64 bytes processed (ignoring whitespace, carriage returns and line feeds), + EVP_DecodeUpdate() produces 3 bytes of binary output data + (except at the end of data terminated with one or two padding characters). + However, the function behaved like an EVP_DecodeBlock(): + produces exactly 3 output bytes for every 4 input bytes. + Such behaviour could cause writes to a non-allocated output buffer + if a user allocates its size based on the documentation and knowing the padding size. + + The fix makes EVP_DecodeUpdate() produce + exactly as many output bytes as in the initial non-encoded message. + + *Valerii Krygin* + OpenSSL 3.4 ----------- diff --git a/crypto/evp/encode.c b/crypto/evp/encode.c index 309d32a35d..4e06657f36 100644 --- a/crypto/evp/encode.c +++ b/crypto/evp/encode.c @@ -19,7 +19,7 @@ static unsigned char conv_ascii2bin(unsigned char a, static int evp_encodeblock_int(EVP_ENCODE_CTX *ctx, unsigned char *t, const unsigned char *f, int dlen); static int evp_decodeblock_int(EVP_ENCODE_CTX *ctx, unsigned char *t, - const unsigned char *f, int n); + const unsigned char *f, int n, int eof); #ifndef CHARSET_EBCDIC # define conv_bin2ascii(a, table) ((table)[(a)&0x3f]) @@ -369,14 +369,14 @@ int EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, } if (n == 64) { - decoded_len = evp_decodeblock_int(ctx, out, d, n); + decoded_len = evp_decodeblock_int(ctx, out, d, n, eof); n = 0; - if (decoded_len < 0 || eof > decoded_len) { + if (decoded_len < 0 || (decoded_len == 0 && eof > 0)) { rv = -1; goto end; } - ret += decoded_len - eof; - out += decoded_len - eof; + ret += decoded_len; + out += decoded_len; } } @@ -388,13 +388,13 @@ int EVP_DecodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl, tail: if (n > 0) { if ((n & 3) == 0) { - decoded_len = evp_decodeblock_int(ctx, out, d, n); + decoded_len = evp_decodeblock_int(ctx, out, d, n, eof); n = 0; - if (decoded_len < 0 || eof > decoded_len) { + if (decoded_len < 0 || (decoded_len == 0 && eof > 0)) { rv = -1; goto end; } - ret += (decoded_len - eof); + ret += decoded_len; } else if (seof) { /* EOF in the middle of a base64 block. */ rv = -1; @@ -411,12 +411,16 @@ end: } static int evp_decodeblock_int(EVP_ENCODE_CTX *ctx, unsigned char *t, - const unsigned char *f, int n) + const unsigned char *f, int n, + int eof) { int i, ret = 0, a, b, c, d; unsigned long l; const unsigned char *table; + if (eof < -1 || eof > 2) + return -1; + if (ctx != NULL && (ctx->flags & EVP_ENCODE_CTX_USE_SRP_ALPHABET) != 0) table = srpdata_ascii2bin; else @@ -437,8 +441,11 @@ static int evp_decodeblock_int(EVP_ENCODE_CTX *ctx, unsigned char *t, if (n % 4 != 0) return -1; + if (n == 0) + return 0; - for (i = 0; i < n; i += 4) { + /* all 4-byte blocks except the last one do not have padding. */ + for (i = 0; i < n - 4; i += 4) { a = conv_ascii2bin(*(f++), table); b = conv_ascii2bin(*(f++), table); c = conv_ascii2bin(*(f++), table); @@ -453,12 +460,43 @@ static int evp_decodeblock_int(EVP_ENCODE_CTX *ctx, unsigned char *t, *(t++) = (unsigned char)(l) & 0xff; ret += 3; } + + /* process the last block that may have padding. */ + a = conv_ascii2bin(*(f++), table); + b = conv_ascii2bin(*(f++), table); + c = conv_ascii2bin(*(f++), table); + d = conv_ascii2bin(*(f++), table); + if ((a | b | c | d) & 0x80) + return -1; + l = ((((unsigned long)a) << 18L) | + (((unsigned long)b) << 12L) | + (((unsigned long)c) << 6L) | (((unsigned long)d))); + + if (eof == -1) + eof = (f[2] == '=') + (f[3] == '='); + + switch (eof) { + case 2: + *(t++) = (unsigned char)(l >> 16L) & 0xff; + break; + case 1: + *(t++) = (unsigned char)(l >> 16L) & 0xff; + *(t++) = (unsigned char)(l >> 8L) & 0xff; + break; + case 0: + *(t++) = (unsigned char)(l >> 16L) & 0xff; + *(t++) = (unsigned char)(l >> 8L) & 0xff; + *(t++) = (unsigned char)(l) & 0xff; + break; + } + ret += 3 - eof; + return ret; } int EVP_DecodeBlock(unsigned char *t, const unsigned char *f, int n) { - return evp_decodeblock_int(NULL, t, f, n); + return evp_decodeblock_int(NULL, t, f, n, 0); } int EVP_DecodeFinal(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl) @@ -467,7 +505,7 @@ int EVP_DecodeFinal(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl) *outl = 0; if (ctx->num != 0) { - i = evp_decodeblock_int(ctx, out, ctx->enc_data, ctx->num); + i = evp_decodeblock_int(ctx, out, ctx->enc_data, ctx->num, -1); if (i < 0) return -1; ctx->num = 0; diff --git a/doc/man3/EVP_EncodeInit.pod b/doc/man3/EVP_EncodeInit.pod index cf6759d505..ab6b07bcdb 100644 --- a/doc/man3/EVP_EncodeInit.pod +++ b/doc/man3/EVP_EncodeInit.pod @@ -176,9 +176,15 @@ EVP_DecodeBlock() returns the length of the data decoded or -1 on error. L +=head1 HISTORY + +The EVP_DecodeUpdate() function was fixed in OpenSSL 3.5, +so now it produces the number of bytes specified in B +and does not decode padding bytes (B<=>) to 6 zero bits. + =head1 COPYRIGHT -Copyright 2016-2024 The OpenSSL Project Authors. All Rights Reserved. +Copyright 2016-2025 The OpenSSL Project Authors. All Rights Reserved. Licensed under the Apache License 2.0 (the "License"). You may not use this file except in compliance with the License. You can obtain a copy diff --git a/test/evp_test.c b/test/evp_test.c index 38626541ec..da80d1843c 100644 --- a/test/evp_test.c +++ b/test/evp_test.c @@ -3383,7 +3383,7 @@ static int encode_test_run(EVP_TEST *t) unsigned char *encode_out = NULL, *decode_out = NULL; int output_len, chunk_len; EVP_ENCODE_CTX *decode_ctx = NULL, *encode_ctx = NULL; - size_t input_len, donelen; + size_t input_len, donelen, decode_length; if (!TEST_ptr(decode_ctx = EVP_ENCODE_CTX_new())) { t->err = "INTERNAL_ERROR"; @@ -3425,9 +3425,14 @@ static int encode_test_run(EVP_TEST *t) goto err; } - if (!TEST_ptr(decode_out = - OPENSSL_malloc(EVP_DECODE_LENGTH(expected->output_len)))) + decode_length = EVP_DECODE_LENGTH(expected->output_len); + if (!TEST_ptr(decode_out = OPENSSL_malloc(decode_length))) goto err; + /* + * Fill memory with non-zeros + * to check that decoding does not place redundant zeros. + */ + memset(decode_out, 0xff, decode_length); output_len = 0; EVP_DecodeInit(decode_ctx); @@ -3463,6 +3468,13 @@ static int encode_test_run(EVP_TEST *t) goto err; } + for (; output_len < (int)decode_length; output_len++) { + if (decode_out[output_len] != 0xff) { + t->err = "BAD_DECODING"; + goto err; + } + } + t->err = NULL; err: OPENSSL_free(encode_out);