diff --git a/embed/boardloader/main.c b/embed/boardloader/main.c index 25cabe45a..7e98ab7bb 100644 --- a/embed/boardloader/main.c +++ b/embed/boardloader/main.c @@ -10,8 +10,8 @@ #include "lowlevel.h" #include "version.h" -#define IMAGE_MAGIC 0x425A5254 // TRZB -#define IMAGE_MAXSIZE (1 * 64 * 1024 + 7 * 128 * 1024) +#define BOOTLOADER_IMAGE_MAGIC 0x425A5254 // TRZB +#define BOOTLOADER_IMAGE_MAXSIZE (1 * 128 * 1024) static uint32_t check_sdcard(void) { @@ -35,7 +35,7 @@ static uint32_t check_sdcard(void) image_header hdr; - if (image_parse_header((const uint8_t *)buf, IMAGE_MAGIC, IMAGE_MAXSIZE, &hdr)) { + if (image_parse_header((const uint8_t *)buf, BOOTLOADER_IMAGE_MAGIC, BOOTLOADER_IMAGE_MAXSIZE, &hdr)) { return hdr.codelen; } else { return 0; @@ -176,7 +176,7 @@ int main(void) image_header hdr; ensure( - image_parse_header((const uint8_t *)BOOTLOADER_START, IMAGE_MAGIC, IMAGE_MAXSIZE, &hdr), + image_parse_header((const uint8_t *)BOOTLOADER_START, BOOTLOADER_IMAGE_MAGIC, BOOTLOADER_IMAGE_MAXSIZE, &hdr), "invalid bootloader header"); ensure( diff --git a/embed/bootloader/main.c b/embed/bootloader/main.c index 237e55a46..244faf0e0 100644 --- a/embed/bootloader/main.c +++ b/embed/bootloader/main.c @@ -17,8 +17,8 @@ #include "messages.h" #include "style.h" -#define IMAGE_MAGIC 0x465A5254 // TRZF -#define IMAGE_MAXSIZE (7 * 128 * 1024) +#define FIRMWARE_IMAGE_MAGIC 0x465A5254 // TRZF +#define FIRMWARE_IMAGE_MAXSIZE (6 * 128 * 1024) void display_fade(int start, int end, int delay) { @@ -266,15 +266,15 @@ int main(void) hal_delay(1); } + vendor_header vhdr; + // start the bootloader if user touched the screen or no firmware installed - if (touched || !vendor_parse_header((const uint8_t *)FIRMWARE_START, NULL)) { + if (touched || !vendor_parse_header((const uint8_t *)FIRMWARE_START, &vhdr)) { if (!bootloader_loop()) { shutdown(); } } - vendor_header vhdr; - ensure( vendor_parse_header((const uint8_t *)FIRMWARE_START, &vhdr), "invalid vendor header"); @@ -286,7 +286,7 @@ int main(void) image_header hdr; ensure( - image_parse_header((const uint8_t *)(FIRMWARE_START + vhdr.hdrlen), IMAGE_MAGIC, IMAGE_MAXSIZE, &hdr), + image_parse_header((const uint8_t *)(FIRMWARE_START + vhdr.hdrlen), FIRMWARE_IMAGE_MAGIC, FIRMWARE_IMAGE_MAXSIZE, &hdr), "invalid firmware header"); ensure( diff --git a/embed/bootloader/messages.c b/embed/bootloader/messages.c index b17b63c18..cf1519bee 100644 --- a/embed/bootloader/messages.c +++ b/embed/bootloader/messages.c @@ -212,7 +212,8 @@ void process_msg_Initialize(uint8_t iface_num, uint32_t msg_size, uint8_t *buf) MSG_SEND_ASSIGN_VALUE(minor_version, VERSION_MINOR); MSG_SEND_ASSIGN_VALUE(patch_version, VERSION_PATCH); MSG_SEND_ASSIGN_VALUE(bootloader_mode, true); - bool firmware_present = vendor_parse_header((const uint8_t *)FIRMWARE_START, NULL); + vendor_header vhdr; + bool firmware_present = vendor_parse_header((const uint8_t *)FIRMWARE_START, &vhdr); MSG_SEND_ASSIGN_VALUE(firmware_present, firmware_present); MSG_SEND(Features); } diff --git a/embed/trezorhal/image.c b/embed/trezorhal/image.c index 5c3fda226..04da31dfb 100644 --- a/embed/trezorhal/image.c +++ b/embed/trezorhal/image.c @@ -29,13 +29,8 @@ static bool compute_pubkey(uint8_t sig_m, uint8_t sig_n, const uint8_t * const * return 0 == ed25519_cosi_combine_publickeys(res, keys, sig_m); } -bool image_parse_header(const uint8_t *data, uint32_t magic, uint32_t maxsize, image_header *hdr) +bool image_parse_header(const uint8_t * const data, const uint32_t magic, const uint32_t maxsize, image_header * const hdr) { - if (!hdr) { - image_header h; - hdr = &h; - } - memcpy(&hdr->magic, data, 4); if (hdr->magic != magic) return false; @@ -43,11 +38,13 @@ bool image_parse_header(const uint8_t *data, uint32_t magic, uint32_t maxsize, i if (hdr->hdrlen != HEADER_SIZE) return false; memcpy(&hdr->expiry, data + 8, 4); + // TODO: expiry mechanism needs to be ironed out before production or those + // devices won't accept expiring bootloaders (due to boardloader write protection). if (hdr->expiry != 0) return false; memcpy(&hdr->codelen, data + 12, 4); - if (hdr->hdrlen + hdr->codelen < 4 * 1024) return false; - if (hdr->hdrlen + hdr->codelen > maxsize) return false; + if (hdr->codelen > (maxsize - hdr->hdrlen)) return false; + if ((hdr->hdrlen + hdr->codelen) < 4 * 1024) return false; if ((hdr->hdrlen + hdr->codelen) % 512 != 0) return false; memcpy(&hdr->version, data + 16, 4); @@ -79,17 +76,13 @@ bool image_check_signature(const uint8_t *data, const image_header *hdr, uint8_t return 0 == ed25519_sign_open(hash, BLAKE2S_DIGEST_LENGTH, pub, *(const ed25519_signature *)hdr->sig); } -bool vendor_parse_header(const uint8_t *data, vendor_header *vhdr) +bool vendor_parse_header(const uint8_t * const data, vendor_header * const vhdr) { - if (!vhdr) { - vendor_header h; - vhdr = &h; - } - memcpy(&vhdr->magic, data, 4); if (vhdr->magic != 0x565A5254) return false; // TRZV memcpy(&vhdr->hdrlen, data + 4, 4); + // TODO: sanity check hdr->hdrlen as it is used as a src to memcpy below memcpy(&vhdr->expiry, data + 8, 4); if (vhdr->expiry != 0) return false; diff --git a/embed/trezorhal/image.h b/embed/trezorhal/image.h index 69d159ec7..c00158b8a 100644 --- a/embed/trezorhal/image.h +++ b/embed/trezorhal/image.h @@ -34,11 +34,11 @@ typedef struct { uint8_t sig[64]; } vendor_header; -bool image_parse_header(const uint8_t *data, uint32_t magic, uint32_t maxsize, image_header *hdr); +bool image_parse_header(const uint8_t * const data, const uint32_t magic, const uint32_t maxsize, image_header * const hdr); bool image_check_signature(const uint8_t *data, const image_header *hdr, uint8_t key_m, uint8_t key_n, const uint8_t * const *keys); -bool vendor_parse_header(const uint8_t *data, vendor_header *vhdr); +bool vendor_parse_header(const uint8_t * const data, vendor_header * const vhdr); bool vendor_check_signature(const uint8_t *data, const vendor_header *vhdr, uint8_t key_m, uint8_t key_n, const uint8_t * const *keys);