espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.44k stars 7.25k forks source link

[TW#18008] Hardware MPI/Bignum causes heap corruption when mbedtls_ecp_mul() #1556

Closed caffeine93 closed 6 years ago

caffeine93 commented 6 years ago

The following code (when hardware MPI is enabled)

`

mbedtls_entropy_context ctxEntropy;
mbedtls_ctr_drbg_context ctxRandom;
const char* pers = "myecdsa";

mbedtls_entropy_init(&ctxEntropy);
mbedtls_ctr_drbg_init(&ctxRandom);
mbedtls_ctr_drbg_seed(&ctxRandom, mbedtls_entropy_func, &ctxEntropy,
        (const unsigned char*) pers, strlen(pers));

mbedtls_ecdsa_context ctxECDSA;
mbedtls_ecdsa_init(&ctxECDSA);
mbedtls_ecp_group_load(&ctxECDSA.grp, MBEDTLS_ECP_DP_SECP256K1);
mbedtls_mpi_copy(&ctxECDSA.d, node->privKey);

mbedtls_ecp_mul(&ctxECDSA.grp, &ctxECDSA.Q, &ctxECDSA.d, &ctxECDSA.grp.G,
        mbedtls_ctr_drbg_random, &ctxRandom);

`

causes heap corruption:

` CORRUPT HEAP: multi_heap.c:369 detected at 0x3ffb3d30 abort() was called at PC 00x40085d48 on core 0 0x40085d48: multi_heap_assert at C:/msys32/home/Admin/esp/esp-idf/components/hea p/multi_heap_platform.h:55 (inlined by) multi_heap_malloc_impl at C:/msys32/home/Admin/esp/esp-idf/compone nts/heap/multi_heap.c:369

Backtrace: 0x400863e8:0x3ffb3c00 0x400864e7:0x3ffb3c20 0x40085d48:0x3ffb3c40 0x4 0081f10:0x3ffb3c60 0x40081f41:0x3ffb3c80 0x40082595:0x3ffb3ca0 0x4000bef5:0x3ffb 3cc0 0x400e25a5:0x3ffb3ce0 0x400e9173:0x3ffb3d00 0x400e72a1:0x3ffb3d30 0x400e3e7 d:0x3ffb3da0 0x400e4d07:0x3ffb3dc0 0x400e5839:0x3ffb3e00 0x400e591e:0x3ffb3e20 0 x400e24c9:0x3ffb3e40 0x400e1e7c:0x3ffb41d0 0x400d15c2:0x3ffb4200 0x400863e8: invoke_abort at C:/msys32/home/Admin/esp/esp-idf/components/esp32/pa nic.c:572

0x400864e7: abort at C:/msys32/home/Admin/esp/esp-idf/components/esp32/panic.c:5 72

0x40085d48: multi_heap_assert at C:/msys32/home/Admin/esp/esp-idf/components/hea p/multi_heap_platform.h:55 (inlined by) multi_heap_malloc_impl at C:/msys32/home/Admin/esp/esp-idf/compone nts/heap/multi_heap.c:369

0x40081f10: heap_caps_malloc at C:/msys32/home/Admin/esp/esp-idf/components/heap /heap_caps.c:116

0x40081f41: heap_caps_malloc_default at C:/msys32/home/Admin/esp/esp-idf/compone nts/heap/heap_caps.c:145

0x40082595: _calloc_r at C:/msys32/home/Admin/esp/esp-idf/components/newlib/sysc alls.c:52

0x400e25a5: mbedtls_mpi_grow at C:/msys32/home/Admin/esp/esp-idf/components/mbed tls/library/bignum.c:2157

0x400e9173: mem_block_to_mpi at C:/msys32/home/Admin/esp/esp-idf/components/mbed tls/port/esp_bignum.c:183 (inlined by) mbedtls_mpi_mul_mpi at C:/msys32/home/Admin/esp/esp-idf/components /mbedtls/port/esp_bignum.c:565

0x400e72a1: ecp_mod_koblitz at C:/msys32/home/Admin/esp/esp-idf/components/mbedt ls/library/ecp_curves.c:1271 (inlined by) ecp_mod_p256k1 at C:/msys32/home/Admin/esp/esp-idf/components/mbed tls/library/ecp_curves.c:1323

0x400e3e7d: ecp_modp at C:/msys32/home/Admin/esp/esp-idf/components/mbedtls/libr ary/ecp.c:660

0x400e4d07: ecp_check_pubkey_sw at C:/msys32/home/Admin/esp/esp-idf/components/m bedtls/library/ecp.c:660

0x400e5839: mbedtls_ecp_check_pubkey at C:/msys32/home/Admin/esp/esp-idf/compone nts/mbedtls/library/ecp.c:1877

0x400e591e: mbedtls_ecp_mul at C:/msys32/home/Admin/esp/esp-idf/components/mbedt ls/library/ecp.c:1688 (discriminator 1)

0x400e24c9: generateMasterNode at C:/Users/Admin/Documents/EmbeddedProjects/ESP3 2/TestProject/main/NGen/NGen.c:302

0x400e1e7c: app_main at C:/Users/Admin/Documents/EmbeddedProjects/ESP32/NGen/ main/main.c:29 `

node is a struct that contains: `

mbedtls_mpi privKey; mbedtls_mpi pubKey;

` both of which are properly allocated prior to the pasted code

Disabling the hardware accelerated MPI in the config menu ("make menuconfig") solves the issue and the multiplication produces the expected result

negativekelvin commented 6 years ago

Something with word alignment perhaps?

projectgus commented 6 years ago

Thanks for the bug report. I was able to reproduce this problem.

The issue is that ecp_mod_koblitz() defines the buffer for an mbedtls_mpi number on the stack, Mp[P_KOBLITZ_MAX + P_KOBLITZ_R + 1]. This is preallocated on the stack because this function knows the maximum size of the result. However, the bignum hardware implementation conservatively tries to grow the result buffer to the size of a hardware bignum result - ie it allocates a new buffer, copies Mp's contents into the new buffer, and then tries to free Mp. The free() fails as the heap implementation sees this is not a heap allocated buffer (a situation which is indistinguishable from heap corruption.)

We will look into fixes for this, there are a few places where this "worst case" allocation takes place in the esp_bignum.c code. In the meantime, the workaround you have (disable hardware MPI) is the fastest way forward.