From 41ec11fb662856334a1bc4a4c956e78c1401682c Mon Sep 17 00:00:00 2001 From: Mark Thompson <129641948+NotherNgineer@users.noreply.github.com> Date: Sat, 27 Jan 2024 06:29:24 -0600 Subject: [PATCH] Check app checksums during untar (#1815) --- .../application/apps/ui_flash_utility.cpp | 20 ++++----- firmware/common/external_app.hpp | 3 +- firmware/common/untar.hpp | 41 +++++++++++++++++-- 3 files changed, 50 insertions(+), 14 deletions(-) diff --git a/firmware/application/apps/ui_flash_utility.cpp b/firmware/application/apps/ui_flash_utility.cpp index b2b30d76..1eb84974 100644 --- a/firmware/application/apps/ui_flash_utility.cpp +++ b/firmware/application/apps/ui_flash_utility.cpp @@ -21,6 +21,7 @@ */ #include "ui_flash_utility.hpp" +#include "ui_styles.hpp" #include "portapack_shared_memory.hpp" namespace ui { @@ -33,13 +34,13 @@ static constexpr size_t max_filename_length = 26; bool valid_firmware_file(std::filesystem::path::string_type path) { File firmware_file; uint32_t read_buffer[128]; - uint32_t checksum{1}; + uint32_t checksum{(uint32_t)~FLASH_EXPECTED_CHECKSUM}; // initializing to invalid checksum in case file can't be read // test read of the whole file just to validate checksum (baseband flash code will re-read when flashing) auto result = firmware_file.open(path.c_str()); if (!result.is_valid()) { checksum = 0; - for (uint32_t i = FLASH_STARTING_ADDRESS; i < FLASH_ROM_SIZE / sizeof(read_buffer); i++) { + for (uint32_t i = 0; i < FLASH_ROM_SIZE / sizeof(read_buffer); i++) { auto readResult = firmware_file.read(&read_buffer, sizeof(read_buffer)); // if file is smaller than 1MB, assume it's a downgrade to an old FW version and ignore the checksum @@ -122,12 +123,6 @@ std::filesystem::path FlashUtilityView::extract_tar(std::filesystem::path::strin painter.fill_rectangle({0, 50, portapack::display.width(), 90}, ui::Color::black()); painter.draw_string({0, 60}, this->nav_.style(), fileName); }); - if (res.string().empty()) { - ui::Painter painter; - painter.fill_rectangle({0, 50, portapack::display.width(), 90}, ui::Color::black()); - painter.draw_string({0, 60}, this->nav_.style(), "BAD TAR FILE"); - chThdSleepMilliseconds(5000); - } return res; } @@ -137,8 +132,13 @@ void FlashUtilityView::flash_firmware(std::filesystem::path::string_type path) { path = extract_tar(u'/' + path).native(); } - if (path.empty() || !valid_firmware_file(path.c_str())) - return; // bad firmware image - just returning back to the file list + if (path.empty() || !valid_firmware_file(path.c_str())) { + ui::Painter painter; + painter.fill_rectangle({0, 50, portapack::display.width(), 90}, ui::Color::black()); + painter.draw_string({0, 60}, Styles::red, "BAD FIRMWARE FILE"); + chThdSleepMilliseconds(5000); + return; + } ui::Painter painter; painter.fill_rectangle( diff --git a/firmware/common/external_app.hpp b/firmware/common/external_app.hpp index 0cae972c..d1d8e771 100644 --- a/firmware/common/external_app.hpp +++ b/firmware/common/external_app.hpp @@ -26,7 +26,8 @@ #include "ui_navigation.hpp" #include "spi_image.hpp" -#define CURRENT_HEADER_VERSION 0x00000001 +#define CURRENT_HEADER_VERSION 0x00000002 +#define MIN_HEADER_VERSION_FOR_CHECKSUM 0x00000002 typedef void (*externalAppEntry_t)(ui::NavigationView& nav); diff --git a/firmware/common/untar.hpp b/firmware/common/untar.hpp index 27cd8130..3e8d2972 100644 --- a/firmware/common/untar.hpp +++ b/firmware/common/untar.hpp @@ -6,6 +6,8 @@ #include #include "string_format.hpp" #include "file.hpp" +#include "utility.hpp" +#include "ui_external_items_menu_loader.hpp" class UnTar { public: @@ -86,6 +88,10 @@ class UnTar { std::string binfile = ""; std::string fn = ""; int filesize; + uint32_t app_checksum; + bool app_file; + bool first_read; + bool corrupt_file{false}; for (;;) { auto readres = a->read(buff, 512); if (!readres.is_ok()) return ""; @@ -132,20 +138,49 @@ class UnTar { delete_file(fn); auto fres = f.open(fn, false, true); if (!fres.value().ok()) return ""; + app_file = (fn.substr(fn.size() - 5) == ".ppma"); + first_read = true; + app_checksum = 0; + corrupt_file = false; } while (filesize > 0) { readres = a->read(buff, 512); - if (!readres.is_ok()) return ""; + if (!readres.is_ok()) { + corrupt_file = true; + break; + } bytes_read = readres.value(); if (bytes_read < 512) { - return ""; + corrupt_file = true; + break; } if (filesize < 512) bytes_read = filesize; + if (app_file && first_read) { + // Check app file header for min version for checksum support + struct application_information_t* app_info = (struct application_information_t*)buff; + if (app_info->header_version < MIN_HEADER_VERSION_FOR_CHECKSUM) + app_file = false; + first_read = false; + } + if (app_file) { + app_checksum += simple_checksum((uint32_t)buff, bytes_read); + } auto fwres = f.write(buff, bytes_read); - if (!fwres.is_ok()) return ""; + if (!fwres.is_ok()) { + corrupt_file = true; + break; + } filesize -= bytes_read; f.sync(); + if ((filesize == 0) && app_file && (app_checksum != EXT_APP_EXPECTED_CHECKSUM)) { + corrupt_file = true; + break; + } + } + if (corrupt_file) { + delete_file(fn); + return ""; } } return binfile;