From 0a4a5feaa0d8f604b6b8284477cd8e1dd798b8fc Mon Sep 17 00:00:00 2001 From: tychovrahe Date: Fri, 1 Mar 2024 20:02:07 +0100 Subject: [PATCH] fix(core): fix TOCTOU in sd card bootloader update procedure [no changelog] --- core/embed/boardloader/main.c | 44 ++++++++++++++---------- core/embed/boardloader/memory_stm32f4.ld | 5 +++ core/embed/lib/buffers.h | 2 +- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/core/embed/boardloader/main.c b/core/embed/boardloader/main.c index 8cbd3871d..1ff87fafb 100644 --- a/core/embed/boardloader/main.c +++ b/core/embed/boardloader/main.c @@ -122,7 +122,7 @@ struct BoardCapabilities capablities .terminator_length = 0}; // we use SRAM as SD card read buffer (because DMA can't access the CCMRAM) -BUFFER_SECTION uint32_t sdcard_buf[IMAGE_HEADER_SIZE / sizeof(uint32_t)]; +BUFFER_SECTION uint32_t sdcard_buf[BOOTLOADER_IMAGE_MAXSIZE / sizeof(uint32_t)]; #if defined USE_SD_CARD static uint32_t check_sdcard(void) { @@ -138,8 +138,8 @@ static uint32_t check_sdcard(void) { memzero(sdcard_buf, IMAGE_HEADER_SIZE); - const secbool read_status = - sdcard_read_blocks(sdcard_buf, 0, IMAGE_HEADER_SIZE / SDCARD_BLOCK_SIZE); + const secbool read_status = sdcard_read_blocks( + sdcard_buf, 0, BOOTLOADER_IMAGE_MAXSIZE / SDCARD_BLOCK_SIZE); sdcard_power_off(); @@ -162,6 +162,21 @@ static uint32_t check_sdcard(void) { return 0; } + _Static_assert(IMAGE_CHUNK_SIZE >= BOOTLOADER_IMAGE_MAXSIZE, + "BOOTLOADER IMAGE MAXSIZE too large for IMAGE_CHUNK_SIZE"); + + if (sectrue != (check_single_hash( + hdr->hashes, ((const uint8_t *)sdcard_buf) + hdr->hdrlen, + hdr->codelen))) { + return 0; + } + + for (int i = IMAGE_HASH_DIGEST_LENGTH; i < sizeof(hdr->hashes); i++) { + if (hdr->hashes[i] != 0) { + return 0; + } + } + #ifdef STM32U5 if (hdr->monotonic < get_bootloader_min_version()) { return 0; @@ -212,24 +227,15 @@ static secbool copy_sdcard(void) { // copy bootloader from SD card to Flash term_printf("copying new bootloader from SD card\n\n"); - ensure(sdcard_power_on(), NULL); - - memzero(sdcard_buf, SDCARD_BLOCK_SIZE); - - for (int i = 0; i < (IMAGE_HEADER_SIZE + codelen) / SDCARD_BLOCK_SIZE; i++) { - ensure(sdcard_read_blocks(sdcard_buf, i, 1), NULL); - for (int j = 0; - j < SDCARD_BLOCK_SIZE / (FLASH_BURST_LENGTH * sizeof(uint32_t)); j++) { - ensure( - flash_area_write_burst( - &BOOTLOADER_AREA, - i * SDCARD_BLOCK_SIZE + j * FLASH_BURST_LENGTH * sizeof(uint32_t), - &sdcard_buf[j * FLASH_BURST_LENGTH]), - NULL); - } + for (int j = 0; j < (IMAGE_HEADER_SIZE + codelen) / + (FLASH_BURST_LENGTH * sizeof(uint32_t)); + j++) { + ensure(flash_area_write_burst(&BOOTLOADER_AREA, + j * FLASH_BURST_LENGTH * sizeof(uint32_t), + &sdcard_buf[j * FLASH_BURST_LENGTH]), + NULL); } - sdcard_power_off(); ensure(flash_lock_write(), NULL); term_printf("\ndone\n\n"); diff --git a/core/embed/boardloader/memory_stm32f4.ld b/core/embed/boardloader/memory_stm32f4.ld index 5dbf852a0..167d720fb 100644 --- a/core/embed/boardloader/memory_stm32f4.ld +++ b/core/embed/boardloader/memory_stm32f4.ld @@ -48,6 +48,11 @@ SECTIONS { . = ALIGN(4); /* make the section size a multiple of the word size */ } >CCMRAM + .buf : ALIGN(4) { + *(.buf*); + . = ALIGN(4); + } >SRAM + /* Hard-coded address for capabilities structure */ .capabilities 0x0800BF00 : {KEEP(*(.capabilities_section))} diff --git a/core/embed/lib/buffers.h b/core/embed/lib/buffers.h index 11a8bc4d9..03d00a79e 100644 --- a/core/embed/lib/buffers.h +++ b/core/embed/lib/buffers.h @@ -45,7 +45,7 @@ // sometimes #define JPEG_WORK_SIZE (3100 + 256 + (6 << 10) + 1000) -#if defined BOOTLOADER +#if defined BOOTLOADER || defined BOARDLOADER #define BUFFER_SECTION __attribute__((section(".buf"))) #else #define BUFFER_SECTION