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

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)

diff --git a/AUTHORS.md b/AUTHORS.md
index f9a290305000554d682948f0c948c1186428d592..5b036a0d8358167c87c7fa275cc671cd222dcce6 100644 (file)
--- 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 67d97e9e6d46f916a103cb44661482dca226343b..d846e789a88a754b708034e9ff740288b30da446 100644 (file)
--- 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 309d32a35d8b0f0e991e968eec0b7d94a8a7b340..4e06657f3620adc34b45c10a25a26c941dd20cd7 100644 (file)
--- 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 cf6759d5052fb84fd0f5078af930e7dbf37dc363..ab6b07bcdba88c70799b7062e0ef1f477d859f80 100644 (file)
--- 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<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

diff --git a/test/evp_test.c b/test/evp_test.c
index 38626541ecd809d5d81951edf3f335beee4b29f5..da80d1843ce9fab70231cfc1da34184334de8144 100644 (file)
--- 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);