diff --git a/legacy/bootloader/.changelog.d/noissue.security b/legacy/bootloader/.changelog.d/noissue.security new file mode 100644 index 000000000..a18706c01 --- /dev/null +++ b/legacy/bootloader/.changelog.d/noissue.security @@ -0,0 +1 @@ +Erase storage when downgrading below fix_version. diff --git a/legacy/bootloader/usb.c b/legacy/bootloader/usb.c index 3698622ea..3253ac508 100644 --- a/legacy/bootloader/usb.c +++ b/legacy/bootloader/usb.c @@ -156,6 +156,51 @@ static secbool readprotobufint(const uint8_t **ptr, uint32_t *result) { return sectrue; } +/** Reverse-endian version comparison + * + * Versions are loaded from the header via a packed struct image_header. A + * version is represented as a single uint32_t. Arm is natively little-endian, + * but the version is actually stored as four bytes in major-minor-patch-build + * order. This function implements `cmp` with "lowest" byte first. + */ +static int version_compare(const uint32_t vera, const uint32_t verb) { + int a, b; // signed temp values so that we can safely return a signed result + a = vera & 0xFF; + b = verb & 0xFF; + if (a != b) return a - b; + a = (vera >> 8) & 0xFF; + b = (verb >> 8) & 0xFF; + if (a != b) return a - b; + a = (vera >> 16) & 0xFF; + b = (verb >> 16) & 0xFF; + if (a != b) return a - b; + a = (vera >> 24) & 0xFF; + b = (verb >> 24) & 0xFF; + return a - b; +} + +static int should_keep_storage(int old_was_signed, + uint32_t fix_version_current) { + // if the current firmware is unsigned, always erase storage + if (SIG_OK != old_was_signed) return SIG_FAIL; + + const image_header *new_hdr = (const image_header *)FW_HEADER; + // if the new header is unsigned, erase storage + if (SIG_OK != signatures_new_ok(new_hdr, NULL)) return SIG_FAIL; + // if the new header hashes don't match flash contents, erase storage + if (SIG_OK != check_firmware_hashes(new_hdr)) return SIG_FAIL; + + // going from old-style header to new-style is always an upgrade, keep storage + if (firmware_present_old()) return SIG_OK; + + // if the current fix_version is higher than the new one, erase storage + if (version_compare(new_hdr->version, fix_version_current) < 0) { + return SIG_FAIL; + } + + return SIG_OK; +} + static void rx_callback(usbd_device *dev, uint8_t ep) { (void)ep; static uint16_t msg_id = 0xFFFF; @@ -163,6 +208,7 @@ static void rx_callback(usbd_device *dev, uint8_t ep) { static uint32_t w; static int wi; static int old_was_signed; + static uint32_t fix_version_current = 0xffffffff; if (usbd_ep_read_packet(dev, ENDPOINT_ADDRESS_OUT, buf, 64) != 64) return; @@ -235,10 +281,13 @@ static void rx_callback(usbd_device *dev, uint8_t ep) { (const image_header *)FLASH_PTR(FLASH_FWHEADER_START); old_was_signed = signatures_new_ok(hdr, NULL) & check_firmware_hashes(hdr); + fix_version_current = hdr->fix_version; } else if (firmware_present_old()) { old_was_signed = signatures_old_ok(); + fix_version_current = 0; } else { old_was_signed = SIG_FAIL; + fix_version_current = 0xffffffff; } erase_code_progress(); send_msg_success(dev); @@ -388,8 +437,7 @@ static void rx_callback(usbd_device *dev, uint8_t ep) { // 1) old firmware was unsigned or not present // 2) signatures are not OK // 3) hashes are not OK - if (SIG_OK != old_was_signed || SIG_OK != signatures_new_ok(hdr, NULL) || - SIG_OK != check_firmware_hashes(hdr)) { + if (SIG_OK != should_keep_storage(old_was_signed, fix_version_current)) { // erase storage erase_storage(); // check erasure