mbedtls_cipher_auth_encrypt with AES key wrap OOB write

[guidovranken]

Description

• Type: Bug

---

Bug

mbed TLS build:
Latest git checkout, built using:

git clone --depth 1 https://github.com/ARMmbed/mbedtls.git
cd mbedtls/
scripts/config.pl set MBEDTLS_PLATFORM_MEMORY
scripts/config.pl set MBEDTLS_CMAC_C
scripts/config.pl set MBEDTLS_NIST_KW_C
scripts/config.pl set MBEDTLS_ARIA_C
scripts/config.pl set MBEDTLS_MD2_C
scripts/config.pl set MBEDTLS_MD4_C
mkdir build/
cd build/
cmake .. -DENABLE_PROGRAMS=0 -DENABLE_TESTING=0
make -j$(nproc)

Expected behavior
No memory violation

Actual behavior
OOB write, as can be verified using Valgrind or AddressSanitizer.

This is not in accordance with the documentation, which states:

The buffer for the output data. This must be able to hold at least ilen Bytes. 

Steps to reproduce

This bug was found by OSS-Fuzz. The following code is a standalone reproducer for https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=25529

#include <stdbool.h>
#include <stdlib.h>

#include <mbedtls/aes.h>
#include <mbedtls/cipher.h>

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }
#define CF_CHECK_NE(expr, res) if ( (expr) == (res) ) { goto end; }

const uint8_t cleartext[] = {0xbf, 0xf1, 0xbb, 0x7a, 0x57, 0x7f, 0x9b, 0x2c, 0xd7, 0x7f, 0xff, 0x7f, 0x00, 0x00, 0x00, 0x01};
const uint8_t key[] = {0x81, 0x00, 0x00, 0x00, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x57, 0x00, 0x7a, 0xb2};

int main(void)
{
    mbedtls_cipher_context_t cipher_ctx;
    const mbedtls_cipher_info_t *cipher_info = NULL;
    bool ctxInited = false;

    uint8_t* out = malloc(19);

    /* Initialize */
    {
        CF_CHECK_NE(cipher_info = mbedtls_cipher_info_from_type(MBEDTLS_CIPHER_AES_128_KW), NULL);
        mbedtls_cipher_init(&cipher_ctx);
        ctxInited = true;
        CF_CHECK_EQ(mbedtls_cipher_setup(&cipher_ctx, cipher_info), 0);
        CF_CHECK_EQ(mbedtls_cipher_setkey(&cipher_ctx, key, sizeof(key) * 8, MBEDTLS_ENCRYPT), 0);
        CF_CHECK_EQ(mbedtls_cipher_reset(&cipher_ctx), 0);
    }

    /* Process/finalize */
    {
        size_t olen;
        CF_CHECK_EQ(mbedtls_cipher_auth_encrypt(&cipher_ctx,
                    NULL, 0,
                    NULL, 0,
                    cleartext, sizeof(cleartext),
                    out, &olen,
                    NULL, 0), 0);
    }

end:
    free(out);

    if ( ctxInited == true ) {
        mbedtls_cipher_free(&cipher_ctx);
    }

    return 0;
}

[gilles-peskine-arm]

Thanks for reporting this! Would you mind sharing which oss-fuzz project caught this? This isn't from the mbedtls project, which we do monitor.

The buffer for the output data. This must be able to hold at least ilen Bytes.

That's going to have to change, because NIST_KW isn't broken down into a “ciphertext” with the same size as the plaintext and a “tag”. I'm not sure the mapping between the mbedtls_cipher API and NIST_KW makes sense here.

But even so the mbedtls_cipher abstraction should have detected this and returned an error, not silently caused a buffer overflow.

[guidovranken]

This was found by my project Cryptofuzz (https://github.com/guidovranken/cryptofuzz) which runs on OSS-Fuzz.

I can add any number of e-mail addresses to the project's CC list if you want (or feel free to do it yourself by changing https://github.com/google/oss-fuzz/blob/master/projects/cryptofuzz/project.yaml through a PR).