diff --git a/ChangeLog.d/base64_decode.txt b/ChangeLog.d/base64_decode.txt index 93dfef12ff..2cd2c598aa 100644 --- a/ChangeLog.d/base64_decode.txt +++ b/ChangeLog.d/base64_decode.txt @@ -1,3 +1,8 @@ Bugfix - * Fix mbedtls_base64_decode() accepting invalid inputs with 4n+1 digits - (the last digit was ignored). + * Fix mbedtls_base64_decode() on inputs that did not have the correct + 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. diff --git a/library/base64.c b/library/base64.c index bff9123398..cc6a73d150 100644 --- a/library/base64.c +++ b/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 @@ -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 - * 4n, 4n+2 or 4n+3. */ + /* In valid base64, the number of digits (n-equals) is always of the form + * 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 == 0) { - *olen = 0; - return 0; + if (n % 4 != 0) { + return MBEDTLS_ERR_BASE64_INVALID_CHARACTER; } - /* The following expression is to calculate the following formula without - * risk of integer overflow in n: - * n = ( ( n * 6 ) + 7 ) >> 3; - */ - n = (6 * (n >> 3)) + ((6 * (n & 0x7) + 7) >> 3); - n -= equals; + /* We've determined that the input is valid, and that it contains + * n digits-plus-trailing-equal-signs, which means (n - equals) digits. + * Now set *olen to the exact length of the output. */ + /* Each block of 4 digits in the input map to 3 bytes of output. + * The last block can have one or two equal signs, in which case + * there are that many fewer output bytes. */ + *olen = (n / 4) * 3 - equals; - if (dst == NULL || dlen < n) { - *olen = n; + if ((*olen != 0 && dst == NULL) || dlen < *olen) { 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; - } else { - x |= mbedtls_ct_base64_dec_value(*src); + /* We already know from the first loop that equal signs are + * only at the end. */ + 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); - } - if (equals <= 0) { - *p++ = MBEDTLS_BYTE_0(x); - } + *p++ = MBEDTLS_BYTE_1(x); + *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; } diff --git a/tests/suites/test_suite_base64.data b/tests/suites/test_suite_base64.data index 4d3b5b9a1a..547b9fd127 100644 --- a/tests/suites/test_suite_base64.data +++ b/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 -# invalid inputs (number of digts + equals not a multiple of 4). -# 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. - +# The next few test cases validate systematically for short inputs that +# we require the correct number of trailing equal signs. 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) -mbedtls_base64_decode:"Yw":"!!":0 +Base64 decode: 2 digits, 0 equals (bad) +mbedtls_base64_decode:"Yw":"c":MBEDTLS_ERR_BASE64_INVALID_CHARACTER -Base64 decode: 2 digits, 1 equals (sloppily accepted) -mbedtls_base64_decode:"Yw=":"!!":0 +Base64 decode: 2 digits, 1 equals (bad) +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) -mbedtls_base64_decode:"Y28":"!!!":0 +Base64 decode: 3 digits, 0 equals (bad) +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) -mbedtls_base64_decode:"Y28==":"co":0 +Base64 decode: 3 digits, 2 equals (bad) +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) -mbedtls_base64_decode:"Y29t=":"com":0 +Base64 decode: 4 digits, 1 equals (bad) +mbedtls_base64_decode:"Y29t=":"com":MBEDTLS_ERR_BASE64_INVALID_CHARACTER -Base64 decode: 4 digits, 2 equals (sloppily accepted) -mbedtls_base64_decode:"Y29t==":"com":0 +Base64 decode: 4 digits, 2 equals (bad) +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) -mbedtls_base64_decode:"Y29tcA":"com!!":0 +Base64 decode: 6 digits, 0 equals (bad) +mbedtls_base64_decode:"Y29tcA":"comp":MBEDTLS_ERR_BASE64_INVALID_CHARACTER -Base64 decode: 6 digits, 1 equals (sloppily accepted) -mbedtls_base64_decode:"Y29tcA=":"com!!":0 +Base64 decode: 6 digits, 1 equals (bad) +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) -mbedtls_base64_decode:"Y29tcG8":"com!!!":0 +Base64 decode: 7 digits, 0 equals (bad) +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) -mbedtls_base64_decode:"Y29tcG8==":"compo":0 +Base64 decode: 7 digits, 2 equals (bad) +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) -mbedtls_base64_decode:"Y29tcG9z=":"compos":0 +Base64 decode: 8 digits, 1 equals (bad) +mbedtls_base64_decode:"Y29tcG9z=":"compos":MBEDTLS_ERR_BASE64_INVALID_CHARACTER -Base64 decode: 8 digits, 2 equals (sloppily accepted) -mbedtls_base64_decode:"Y29tcG9z==":"compos":0 +Base64 decode: 8 digits, 2 equals (bad) +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 diff --git a/tests/suites/test_suite_base64.function b/tests/suites/test_suite_base64.function index 182be2916f..8c948b4b57 100644 --- a/tests/suites/test_suite_base64.function +++ b/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 */