wolfSSL / wolfssl

The wolfSSL library is a small, fast, portable implementation of TLS/SSL for embedded devices to the cloud. wolfSSL supports up to TLS 1.3 and DTLS 1.3!
https://www.wolfssl.com
GNU General Public License v2.0
2.29k stars 817 forks source link

[Bug]: valgrind-3.15.0 reports "Source and destination overlap in memcpy" when using --enable-aesgcm-stream #6413

Closed YushengYangRTI closed 1 year ago

YushengYangRTI commented 1 year ago

Contact Details

yusheng@rti.com

Version

5.5.1

Description

We built wolfSSL 5.5.1 with the following options:

wolfssl-5.5.1-commercial/configure '--enable-smime' '--enable-opensslall' '--enable-opensslextra' '--enable-crl' '--enable-certgen' '--enable-des3' '--enable-reproducible-build' '--enable-certext' '--enable-aesgcm-stream' '--enable-debug' '--enable-static' '--disable-shared' '--enable-aesni' 

Our target and build environment:

    os=Linux

    os_build=Linux

    arch=x86_64

    arch_build=x86_64

    compiler=gcc

    compiler.version=4.8

    compiler.libcxx=libstdc++

        valgrind version: valgrind-3.15.0

When we run the following C code:

int main(int argc, char **argv)
{
    int retval = -1;
    EVP_CIPHER_CTX *ctx = EVP_CIPHER_CTX_new();

    const unsigned char sessionKey[] = {
        0x2f, 0x38, 0xde, 0x36, 0xd4, 0xbe, 0x5b, 0xfe,
        0xf, 0xb6, 0xa7, 0x59, 0x87, 0x9e, 0xd9, 0x37
    };
    unsigned char iv[] = {
        0x0, 0x0, 0x0, 0x1, 0x5b, 0x8e, 0xc9, 0xf9,
        0xc0, 0xb9, 0xba, 0x28
    };
    unsigned char commonMac[] = {
        0x3b, 0xbe, 0xde, 0x30, 0x6f, 0x30, 0xf1, 0x8,
        0x39, 0x74, 0x8a, 0x99, 0xf9, 0x61, 0x1b, 0xdc
    };
    unsigned char ciphertext[] = {
        0x49, 0xdb, 0x4a, 0x52, 0xf2, 0x65, 0xc4, 0x38,
        0xc8, 0xa0, 0x91, 0xe4, 0x8f, 0x4f, 0x2c, 0xf,
        0x62, 0x14, 0x4, 0x64, 0x46, 0x9e, 0xc, 0x85,
        0xb1, 0xac, 0x1, 0x3a, 0xcc, 0x81, 0x3a, 0x1,
        0xba, 0xe0, 0x27, 0x26, 0x2e, 0x8a, 0x79, 0x54,
        0x65, 0x22, 0x6, 0xc9, 0x1, 0x97, 0x95, 0x72,
        0x17, 0xe9, 0x12, 0xa9, 0x2b, 0x74, 0xb6, 0x84,
        0xf7, 0xb4, 0x89, 0x7b, 0x50, 0xbf, 0x7e, 0xa6,
        0xd0, 0x15, 0xa7, 0xaf, 0x94, 0xb2, 0x71, 0x72,
        0xa, 0xac, 0xf7, 0xe1, 0x42, 0xf1, 0xcd, 0x9a,
        0xed, 0x1e, 0xb8, 0x1d, 0x65, 0xb7, 0xc5, 0x7c,
        0x4a, 0xaa, 0x1f, 0xc6, 0xf5, 0x53, 0xb, 0xf8,
        0x91, 0xfb, 0xd1, 0x29, 0xec, 0x9a, 0x94, 0x70,
        0xa, 0x26, 0xc3, 0xdd, 0xcd, 0x87, 0xb8, 0x1b,
        0xc2, 0xbe, 0x78, 0xbf, 0xbe, 0xb8, 0x3d, 0xfa,
        0x87, 0x88, 0x98, 0xa6, 0xc, 0x58, 0x1, 0x3d,
        0xf6, 0x97, 0x90, 0x53, 0xff, 0xae, 0x5f, 0x16,
        0x3d, 0x28, 0x63, 0x6d, 0xda, 0x90, 0x59, 0xd6,
        0x7, 0x93, 0x67, 0x45, 0x27, 0x46, 0xd7, 0xdf,
        0xf2, 0x70, 0xec, 0x66, 0x9a, 0x19, 0x9d, 0x92,
        0x18, 0x23, 0xc0, 0x2c, 0x4f, 0xb3, 0x18, 0x2f,
        0xf8, 0x6f, 0x9e, 0xce, 0x8c, 0xc3, 0x82, 0xf4,
        0x46, 0xcf, 0x51, 0xdc, 0x6d, 0x77, 0x6b, 0x65,
        0x68, 0x24, 0xaa, 0x11, 0xea, 0xfa, 0xbf, 0x1a,
        0xc9, 0xd6, 0x73, 0xa9, 0x1, 0xd, 0xac, 0xe1,
        0xd8, 0xb6, 0xd5, 0xfa, 0x4e, 0xea, 0xe6, 0xfb,
        0x3b, 0x24, 0xb6, 0x8f, 0x46, 0x85, 0xc6, 0xa7
    };

    unsigned char decryptOutput[512];
    int outputLength = 0;

    if (!EVP_DecryptInit_ex(
            ctx,
            EVP_aes_128_gcm(),
            NULL,
            sessionKey,
            iv)) {
        printf("%s:%d\n", __FILE__, __LINE__);
        goto done;
    }
    if (EVP_CIPHER_CTX_ctrl(
            ctx,
            EVP_CTRL_GCM_SET_TAG,
            16,
            commonMac) != 1) {
        printf("%s:%d\n", __FILE__, __LINE__);
        goto done;
    }
    if (!EVP_DecryptUpdate(
            ctx,
            decryptOutput,
            &outputLength,
            ciphertext,
            sizeof(ciphertext))) {
        printf("%s:%d\n", __FILE__, __LINE__);
        goto done;
    }
    if (EVP_DecryptFinal_ex(
            ctx,
            decryptOutput + outputLength,
            &outputLength) <= 0) {
        printf("%s:%d\n", __FILE__, __LINE__);
        goto done;
    }

    retval = 0;
done:

    EVP_CIPHER_CTX_free(ctx);
    printf(
            "%s:%d: test %s\n",
            __FILE__,
            __LINE__,
            retval == 0 ? "passed" : "failed!!!");
    return retval;
}

we get the following result:

[.venv_x64Linux3gcc4.8.2] {yusheng@x64lin-gcc48-dev:62} /usr/bin/valgrind tester
==6687== Memcheck, a memory error detector
==6687== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==6687== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==6687== Command: security.1.0/lib/x64Linux3gcc4.8.2/sec0_wolfssl_5_5Testerd
==6687== 
==6687== Source and destination overlap in memcpy(0x5b2f170, 0x5b2f170, 12)
==6687==    at 0x4C2E81D: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1035)
==6687==    by 0x1EA5188: wc_AesGcmInit (aes.c:9311)
==6687==    by 0x1E0B451: wolfSSL_EVP_CipherFinal (evp.c:920)
==6687==    by 0x87EE38: main (Tester.c:711)
==6687== 
Tester.c:92: test passed
==6687== 
==6687== HEAP SUMMARY:
==6687==     in use at exit: 0 bytes in 0 blocks
==6687==   total heap usage: 1 allocs, 1 frees, 1,152 bytes allocated
==6687== 
==6687== All heap blocks were freed -- no leaks are possible
==6687== 
==6687== For lists of detected and suppressed errors, rerun with: -s
==6687== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

aes.c:9311 is this:

        if (iv != NULL) {
            /* Cache the IV in AES GCM object. */
            XMEMCPY((byte*)aes->reg, iv, ivSz);
            aes->nonceSz = ivSz;
        }

Is there a version of wolfSSL that fixes this problem?

Thanks, Yusheng

Reproduction steps

No response

Relevant log output

No response

embhorn commented 1 year ago

Hello @YushengYangRTI

Thanks for contacting wolfSSL Support. There have been some fixes in aes.c since v5.5.1 was released. Could you please test with the latest code and let us know if the warning still appears?

Thanks, @embhorn - wolfSSL Support

YushengYangRTI commented 1 year ago

Hi @embhorn ,

Thanks for your response. I tried v5.6.0, and I got a similar error:

==8620== Source and destination overlap in memcpy(0x5b2f170, 0x5b2f170, 12)
==8620==    at 0x4C2E81D: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1035)
==8620==    by 0x4C112B: wc_AesGcmInit (aes.c:8178)
==8620==    by 0x424E90: wolfSSL_EVP_CipherFinal (evp.c:1019)
==8620==    by 0x4243F1: wolfSSL_EVP_DecryptFinal_ex (evp.c:415)
==8620==    by 0x4236E7: main (in 6413)

aes.c:8178 is this:

        /* Set the IV passed in if it is smaller than a block. */
        if ((iv != NULL) && (ivSz <= AES_BLOCK_SIZE)) {
            XMEMCPY((byte*)aes->reg, iv, ivSz);
            aes->nonceSz = ivSz;
        }
embhorn commented 1 year ago

Hello @YushengYangRTI

Could you please retest with this PR (#6416) to confirm you see the issue is resolved?

embhorn commented 1 year ago

Just FYI, I was not able to reproduce the issue with a newer version of valgrind:

 valgrind --version
valgrind-3.18.1
YushengYangRTI commented 1 year ago

Hi @embhorn ,

PR 6416 fixed the problem for me.

Thanks, Yusheng

colmenero commented 1 year ago

In addition, it may be nice to avoid calling wc_AesGcmInit when the iv argument points to aes->reg (example). That is, instead of:

if (iv != NULL) {
    /* Cache the IV in AES GCM object. */
    XMEMMOVE((byte*)aes->reg, iv, ivSz);
    aes->nonceSz = ivSz;
}

do:

if (iv != NULL && iv != aes->reg) {
    /* Cache the IV in AES GCM object. */
    XMEMMOVE((byte*)aes->reg, iv, ivSz);
    aes->nonceSz = ivSz;
}