mbedtls_base64_decode: insist on correct padding

Correct base64 input (excluding ignored characters such as spaces) consists
of exactly 4*k, 4*k-1 or 4*k-2 digits, followed by 0, 1 or 2 equal signs
respectively.

Previously, any number of trailing equal signs up to 2 was accepted, but if
there fewer than 4*k digits-or-equals, the last partial block was counted in
`*olen` in buffer-too-small mode, but was not output despite returning 0.

Now `mbedtls_base64_decode()` insists on correct padding. This is
backward-compatible since the only plausible useful inputs that used to be
accepted were inputs with 4*k-1 or 4*k-2 digits and no trailing equal signs,
and those led to invalid (truncated) output. Furthermore the function now
always reports the exact output size in buffer-too-small mode.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

ChangeLog.d/base64_decode.txt

@@ -1,3 +1,8 @@
 Bugfix
-   * Fix mbedtls_base64_decode() accepting invalid inputs with 4n+1 digits
+   * Fix mbedtls_base64_decode() on inputs that did not have the correct
-     (the last digit was ignored).
+     number of trailing equal signs, or had 4*k+1 digits. They were accepted
+     as long as they had at most two trailing equal signs. They are now
+     rejected. Furthermore, before, on inputs with too few equal signs, the
+     function reported the correct size in *olen when it returned
+     MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL, but truncated the output to the
+     last multiple of 3 bytes.

library/base64.c

@@ -14,6 +14,7 @@
 #include "mbedtls/base64.h"
 #include "base64_internal.h"
 #include "constant_time_internal.h"
+#include "mbedtls/error.h"
 
 #include <stdint.h>
 
@@ -183,55 +184,57 @@ int mbedtls_base64_decode(unsigned char *dst, size_t dlen, size_t *olen,
         n++;
     }
 
-    /* In valid base64, the number of digits is always of the form
+    /* In valid base64, the number of digits (n-equals) is always of the form
-     * 4n, 4n+2 or 4n+3. */
+     * 4*k, 4*k+2 or *4k+3. Also, the number n of digits plus the number of
+     * equal signs at the end is always a multiple of 4. */
     if ((n - equals) % 4 == 1) {
         return MBEDTLS_ERR_BASE64_INVALID_CHARACTER;
     }
-
+    if (n % 4 != 0) {
-    if (n == 0) {
+        return MBEDTLS_ERR_BASE64_INVALID_CHARACTER;
-        *olen = 0;
-        return 0;
     }
 
-    /* The following expression is to calculate the following formula without
+    /* We've determined that the input is valid, and that it contains
-     * risk of integer overflow in n:
+     * n digits-plus-trailing-equal-signs, which means (n - equals) digits.
-     *     n = ( ( n * 6 ) + 7 ) >> 3;
+     * Now set *olen to the exact length of the output. */
-     */
+    /* Each block of 4 digits in the input map to 3 bytes of output.
-    n = (6 * (n >> 3)) + ((6 * (n & 0x7) + 7) >> 3);
+     * The last block can have one or two equal signs, in which case
-    n -= equals;
+     * there are that many fewer output bytes. */
+    *olen = (n / 4) * 3 - equals;
 
-    if (dst == NULL || dlen < n) {
+    if ((*olen != 0 && dst == NULL) || dlen < *olen) {
-        *olen = n;
         return MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL;
     }
 
-    equals = 0;
     for (x = 0, p = dst; i > 0; i--, src++) {
         if (*src == '\r' || *src == '\n' || *src == ' ') {
             continue;
         }
-
-        x = x << 6;
         if (*src == '=') {
-            ++equals;
+            /* We already know from the first loop that equal signs are
-        } else {
+             * only at the end. */
-            x |= mbedtls_ct_base64_dec_value(*src);
+            break;
         }
+        x = x << 6;
+        x |= mbedtls_ct_base64_dec_value(*src);
 
         if (++accumulated_digits == 4) {
             accumulated_digits = 0;
             *p++ = MBEDTLS_BYTE_2(x);
-            if (equals <= 1) {
+            *p++ = MBEDTLS_BYTE_1(x);
-                *p++ = MBEDTLS_BYTE_1(x);
+            *p++ = MBEDTLS_BYTE_0(x);
-            }
-            if (equals <= 0) {
-                *p++ = MBEDTLS_BYTE_0(x);
-            }
         }
     }
+    if (accumulated_digits == 3) {
+        *p++ = MBEDTLS_BYTE_2(x << 6);
+        *p++ = MBEDTLS_BYTE_1(x << 6);
+    } else if (accumulated_digits == 2) {
+        *p++ = MBEDTLS_BYTE_2(x << 12);
+    }
 
-    *olen = (size_t) (p - dst);
+    if (*olen != (size_t) (p - dst)) {
+        return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+    }
 
     return 0;
 }

tests/suites/test_suite_base64.data

@@ -76,31 +76,25 @@ mbedtls_base64_decode:"zm=masd":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 Base64 decode (Space inside string)
 mbedtls_base64_decode:"zm masd":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-# Validate the weird behavior of mbedtls_base64_decode() on some
+# The next few test cases validate systematically for short inputs that
-# invalid inputs (number of digts + equals not a multiple of 4).
+# we require the correct number of trailing equal signs.
-# In the reference output, "!" characters at the end are needed to
-# pad the output buffer, but the actual output omits those. E.g. if
-# dst_string is "ab!" then mbedtls_base64_decode() reports a 3-byte
-# output when dlen < 3, but actually outputs 2 bytes if given a
-# buffer of 3 bytes or more.
-
 Base64 decode: 1 digit, 0 equals (bad)
 mbedtls_base64_decode:"Y":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 1 digit, 1 equals (bad)
-mbedtls_base64_decode:"Y":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
+mbedtls_base64_decode:"Y=":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 1 digit, 2 equals (bad)
 mbedtls_base64_decode:"Y==":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 1 digit, 3 equals (bad)
-mbedtls_base64_decode:"Y===":"!":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
+mbedtls_base64_decode:"Y===":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 2 digits, 0 equals (sloppily accepted)
+Base64 decode: 2 digits, 0 equals (bad)
-mbedtls_base64_decode:"Yw":"!!":0
+mbedtls_base64_decode:"Yw":"c":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 2 digits, 1 equals (sloppily accepted)
+Base64 decode: 2 digits, 1 equals (bad)
-mbedtls_base64_decode:"Yw=":"!!":0
+mbedtls_base64_decode:"Yw=":"c":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 2 digits, 2 equals (good)
 mbedtls_base64_decode:"Yw==":"c":0
@@ -108,14 +102,14 @@ mbedtls_base64_decode:"Yw==":"c":0
 Base64 decode: 2 digits, 3 equals (bad)
 mbedtls_base64_decode:"Yw===":"c":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 3 digits, 0 equals (sloppily accepted)
+Base64 decode: 3 digits, 0 equals (bad)
-mbedtls_base64_decode:"Y28":"!!!":0
+mbedtls_base64_decode:"Y28":"co":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 3 digits, 1 equals (good)
 mbedtls_base64_decode:"Y28=":"co":0
 
-Base64 decode: 3 digits, 2 equals (sloppily accepted)
+Base64 decode: 3 digits, 2 equals (bad)
-mbedtls_base64_decode:"Y28==":"co":0
+mbedtls_base64_decode:"Y28==":"co":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 3 digits, 3 equals (bad)
 mbedtls_base64_decode:"Y28===":"co":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
@@ -123,11 +117,11 @@ mbedtls_base64_decode:"Y28===":"co":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 Base64 decode: 4 digits, 0 equals (good)
 mbedtls_base64_decode:"Y29t":"com":0
 
-Base64 decode: 4 digits, 1 equals (sloppily accepted)
+Base64 decode: 4 digits, 1 equals (bad)
-mbedtls_base64_decode:"Y29t=":"com":0
+mbedtls_base64_decode:"Y29t=":"com":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 4 digits, 2 equals (sloppily accepted)
+Base64 decode: 4 digits, 2 equals (bad)
-mbedtls_base64_decode:"Y29t==":"com":0
+mbedtls_base64_decode:"Y29t==":"com":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 4 digits, 3 equals (bad)
 mbedtls_base64_decode:"Y29t===":"com":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
@@ -142,13 +136,13 @@ Base64 decode: 5 digits, 2 equals (bad)
 mbedtls_base64_decode:"Y29tc==":"":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 5 digits, 3 equals (bad)
-mbedtls_base64_decode:"Y29tc===":"com!":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
+mbedtls_base64_decode:"Y29tc===":"com":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 6 digits, 0 equals (sloppily accepted)
+Base64 decode: 6 digits, 0 equals (bad)
-mbedtls_base64_decode:"Y29tcA":"com!!":0
+mbedtls_base64_decode:"Y29tcA":"comp":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 6 digits, 1 equals (sloppily accepted)
+Base64 decode: 6 digits, 1 equals (bad)
-mbedtls_base64_decode:"Y29tcA=":"com!!":0
+mbedtls_base64_decode:"Y29tcA=":"comp":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 6 digits, 2 equals (good)
 mbedtls_base64_decode:"Y29tcA==":"comp":0
@@ -156,14 +150,14 @@ mbedtls_base64_decode:"Y29tcA==":"comp":0
 Base64 decode: 6 digits, 3 equals (bad)
 mbedtls_base64_decode:"Y29tcA===":"comp":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 7 digits, 0 equals (sloppily accepted)
+Base64 decode: 7 digits, 0 equals (bad)
-mbedtls_base64_decode:"Y29tcG8":"com!!!":0
+mbedtls_base64_decode:"Y29tcG8":"compo":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 7 digits, 1 equals (good)
 mbedtls_base64_decode:"Y29tcG8=":"compo":0
 
-Base64 decode: 7 digits, 2 equals (sloppily accepted)
+Base64 decode: 7 digits, 2 equals (bad)
-mbedtls_base64_decode:"Y29tcG8==":"compo":0
+mbedtls_base64_decode:"Y29tcG8==":"compo":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 7 digits, 3 equals (bad)
 mbedtls_base64_decode:"Y29tcG8===":"compo":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
@@ -171,11 +165,11 @@ mbedtls_base64_decode:"Y29tcG8===":"compo":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 Base64 decode: 8 digits, 0 equals (good)
 mbedtls_base64_decode:"Y29tcG9z":"compos":0
 
-Base64 decode: 8 digits, 1 equals (sloppily accepted)
+Base64 decode: 8 digits, 1 equals (bad)
-mbedtls_base64_decode:"Y29tcG9z=":"compos":0
+mbedtls_base64_decode:"Y29tcG9z=":"compos":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
-Base64 decode: 8 digits, 2 equals (sloppily accepted)
+Base64 decode: 8 digits, 2 equals (bad)
-mbedtls_base64_decode:"Y29tcG9z==":"compos":0
+mbedtls_base64_decode:"Y29tcG9z==":"compos":MBEDTLS_ERR_BASE64_INVALID_CHARACTER
 
 Base64 decode: 8 digits, 3 equals (bad)
 mbedtls_base64_decode:"Y29tcG9z===":"compos":MBEDTLS_ERR_BASE64_INVALID_CHARACTER

tests/suites/test_suite_base64.function

@@ -99,25 +99,12 @@ void mbedtls_base64_decode(char *src_string, char *dst_string, int result)
 
     TEST_CALLOC(dst, dst_size);
 
-    /* Validate broken behavior observed on Mbed TLS 3.6.3:
-     * some invalid inputs are accepted, and asking for the decoded length
-     * gives a figure that's longer than the decoded output.
-     * In the test data, trailing "!" characters in dst_string indicate
-     * padding that must be present in the output buffer length, but
-     * will not be present in the actual output when the output buffer
-     * is large enough.
-     */
-    size_t expected_dst_len = correct_dst_len;
-    while (expected_dst_len > 0 && dst_string[expected_dst_len - 1] == '!') {
-        --expected_dst_len;
-    }
-
     /* Test normal operation */
     TEST_EQUAL(mbedtls_base64_decode(dst, dst_size, &len,
                                      src, src_len),
                result);
     if (result == 0) {
-        TEST_MEMORY_COMPARE(dst_string, expected_dst_len, dst, len);
+        TEST_MEMORY_COMPARE(dst_string, correct_dst_len, dst, len);
     }
 
     /* Test an output buffer that's one byte too small */