mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2025-03-31 20:10:45 +08:00
Code Issues Packages Projects Releases Wiki Activity

EVP_DecodeUpdate() should not produce padding zeros to the decoded output (Fixes #26677)

Browse Source 
EVP_DecodeUpdate() should not produce zeros for input padding `=` signs to avoid writing to non-allocated memory regions.

To achieve this:
- Add `eof` parameter to `evp_decodeblock_int` function in `openssl/crypto/evp`. The parameter should either contain the number of the input padding characters to ignore or `-1` if the function has to count them.
- Use precalculated `eof` in `EVP_DecodeUpdate` to fix its behaviour.
- Use `eof = -1` in `EVP_DecodeFinal` to count it in `evp_decodeblock_int`.
- Do not ignore padding in `EVP_DecodeBlock` (`eof = 0`) because it should write padding zeros according to the documentation.
- Add the HISTORY section to EVP_EncodeInit documentation to describe the fix.

Other changes:
- Update AUTHORS.md
- Update the copyright date in the documentation.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/26678)
This commit is contained in:
Valerii Krygin 2025-02-25 15:57:26 +00:00
parent 6ef393b89b
commit f86acc9434
5 changed files with 89 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
AUTHORS.md
View File

@ -49,4 +49,5 @@ Individuals
* Tim Hudson
* Tomáš Mráz
* Ulf Möller
* Valerii Krygin
* Viktor Dukhovni

16
CHANGES.md
View File

@ -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
-----------

62
crypto/evp/encode.c
View File

@ -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;

8
doc/man3/EVP_EncodeInit.pod
View File

@ -176,9 +176,15 @@ EVP_DecodeBlock() returns the length of the data decoded or -1 on error.
L<evp(7)>
=head1 HISTORY
The EVP_DecodeUpdate() function was fixed in OpenSSL 3.5,
so now it produces the number of bytes specified in B<outl*>
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

18
test/evp_test.c
View File

@ -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);