From 3e28724b6262ab64dca9cb283c011419cf7574f5 Mon Sep 17 00:00:00 2001 From: IFAKA <99131130+IFAKA@users.noreply.github.com> Date: Fri, 19 Dec 2025 13:23:23 +0100 Subject: [PATCH 1/6] Add bounds checking for TOC/spine array access (#64) ## Problem `getSpineIndexForTocIndex()` and `getTocIndexForSpineIndex()` access `toc[tocIndex]` and `spine[spineIndex]` without validating indices are within bounds. Malformed EPUBs or edge cases could trigger out-of-bounds access. ## Fix Added bounds validation at the start of both functions before accessing the arrays. ## Testing - Builds successfully with `pio run` - Affects: `lib/Epub/Epub.cpp` --- lib/Epub/Epub.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/Epub/Epub.cpp b/lib/Epub/Epub.cpp index 1477d720..408ebf8c 100644 --- a/lib/Epub/Epub.cpp +++ b/lib/Epub/Epub.cpp @@ -322,6 +322,11 @@ int Epub::getTocItemsCount() const { return toc.size(); } // work out the section index for a toc index int Epub::getSpineIndexForTocIndex(const int tocIndex) const { + if (tocIndex < 0 || tocIndex >= toc.size()) { + Serial.printf("[%lu] [EBP] getSpineIndexForTocIndex: tocIndex %d out of range\n", millis(), tocIndex); + return 0; + } + // the toc entry should have an href that matches the spine item // so we can find the spine index by looking for the href for (int i = 0; i < spine.size(); i++) { @@ -336,6 +341,11 @@ int Epub::getSpineIndexForTocIndex(const int tocIndex) const { } int Epub::getTocIndexForSpineIndex(const int spineIndex) const { + if (spineIndex < 0 || spineIndex >= spine.size()) { + Serial.printf("[%lu] [EBP] getTocIndexForSpineIndex: spineIndex %d out of range\n", millis(), spineIndex); + return -1; + } + // the toc entry should have an href that matches the spine item // so we can find the toc index by looking for the href for (int i = 0; i < toc.size(); i++) { From 1a53dccebd126ea4c2cc2e593667c64d9954dc57 Mon Sep 17 00:00:00 2001 From: IFAKA <99131130+IFAKA@users.noreply.github.com> Date: Fri, 19 Dec 2025 13:23:43 +0100 Subject: [PATCH 2/6] Fix title truncation crash for short titles (#63) ## Problem The status bar title truncation loop crashes when the chapter title is shorter than 8 characters. ```cpp // title.length() - 8 underflows when length < 8 (size_t is unsigned) title = title.substr(0, title.length() - 8) + "..."; ``` ## Fix Added a length guard to skip truncation for titles that are too short to truncate safely. ## Testing - Builds successfully with `pio run` - Affects: `src/activities/reader/EpubReaderActivity.cpp` --- src/activities/reader/EpubReaderActivity.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index f4e45363..3bae96d6 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -382,7 +382,7 @@ void EpubReaderActivity::renderStatusBar() const { const auto tocItem = epub->getTocItem(tocIndex); title = tocItem.title; titleWidth = renderer.getTextWidth(SMALL_FONT_ID, title.c_str()); - while (titleWidth > availableTextWidth) { + while (titleWidth > availableTextWidth && title.length() > 11) { title = title.substr(0, title.length() - 8) + "..."; titleWidth = renderer.getTextWidth(SMALL_FONT_ID, title.c_str()); } From 48249fbd1e61dec8db36fd14f461a0054711d8e4 Mon Sep 17 00:00:00 2001 From: IFAKA <99131130+IFAKA@users.noreply.github.com> Date: Fri, 19 Dec 2025 13:24:25 +0100 Subject: [PATCH 3/6] Check SD card initialization and show error on failure (#65) ## Problem `SD.begin()` return value was ignored. If the SD card fails to initialize, the device continues and crashes when trying to load settings/state. ## Fix Check return value and display "SD card error" message instead of proceeding with undefined state. ## Testing - Builds successfully with `pio run` - Affects: `src/main.cpp` --- src/main.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index d12c701c..53701f43 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -177,7 +177,12 @@ void setup() { enterNewActivity(new BootActivity(renderer, inputManager)); // SD Card Initialization - SD.begin(SD_SPI_CS, SPI, SPI_FQ); + if (!SD.begin(SD_SPI_CS, SPI, SPI_FQ)) { + Serial.printf("[%lu] [ ] SD card initialization failed\n", millis()); + exitActivity(); + enterNewActivity(new FullScreenMessageActivity(renderer, inputManager, "SD card error", BOLD)); + return; + } SETTINGS.loadFromFile(); APP_STATE.loadFromFile(); From 2d3928ed819862690becf015be560439a0b053c2 Mon Sep 17 00:00:00 2001 From: IFAKA <99131130+IFAKA@users.noreply.github.com> Date: Fri, 19 Dec 2025 13:27:08 +0100 Subject: [PATCH 4/6] Validate file handle when reading progress.bin (#66) ## Problem Reading progress.bin used `SD.exists()` then `SD.open()` without checking if open succeeded. Race conditions or SD errors could cause file handle to be invalid. ## Fix - Removed redundant `SD.exists()` check - Check if file opened successfully before reading - Verify correct number of bytes were read ## Testing - Builds successfully with `pio run` - Affects: `src/activities/reader/EpubReaderActivity.cpp` --- src/activities/reader/EpubReaderActivity.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/activities/reader/EpubReaderActivity.cpp b/src/activities/reader/EpubReaderActivity.cpp index 3bae96d6..dd6dd04a 100644 --- a/src/activities/reader/EpubReaderActivity.cpp +++ b/src/activities/reader/EpubReaderActivity.cpp @@ -33,13 +33,14 @@ void EpubReaderActivity::onEnter() { epub->setupCacheDir(); - if (SD.exists((epub->getCachePath() + "/progress.bin").c_str())) { - File f = SD.open((epub->getCachePath() + "/progress.bin").c_str()); + File f = SD.open((epub->getCachePath() + "/progress.bin").c_str()); + if (f) { uint8_t data[4]; - f.read(data, 4); - currentSpineIndex = data[0] + (data[1] << 8); - nextPageNumber = data[2] + (data[3] << 8); - Serial.printf("[%lu] [ERS] Loaded cache: %d, %d\n", millis(), currentSpineIndex, nextPageNumber); + if (f.read(data, 4) == 4) { + currentSpineIndex = data[0] + (data[1] << 8); + nextPageNumber = data[2] + (data[3] << 8); + Serial.printf("[%lu] [ERS] Loaded cache: %d, %d\n", millis(), currentSpineIndex, nextPageNumber); + } f.close(); } From adfeee063faccbfaa248571e9fd8ab8d2f712796 Mon Sep 17 00:00:00 2001 From: IFAKA <99131130+IFAKA@users.noreply.github.com> Date: Fri, 19 Dec 2025 13:28:36 +0100 Subject: [PATCH 5/6] Handle empty spine in getBookSize() and calculateProgress() (#67) ## Problem - `getBookSize()` calls `getCumulativeSpineItemSize(getSpineItemsCount() - 1)` which passes -1 when spine is empty - `calculateProgress()` then divides by zero when book size is 0 ## Fix - Return 0 from `getBookSize()` if spine is empty - Return 0 from `calculateProgress()` if book size is 0 ## Testing - Builds successfully with `pio run` - Affects: `lib/Epub/Epub.cpp` --- lib/Epub/Epub.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/Epub/Epub.cpp b/lib/Epub/Epub.cpp index 408ebf8c..a3edac84 100644 --- a/lib/Epub/Epub.cpp +++ b/lib/Epub/Epub.cpp @@ -358,13 +358,21 @@ int Epub::getTocIndexForSpineIndex(const int spineIndex) const { return -1; } -size_t Epub::getBookSize() const { return getCumulativeSpineItemSize(getSpineItemsCount() - 1); } +size_t Epub::getBookSize() const { + if (spine.empty()) { + return 0; + } + return getCumulativeSpineItemSize(getSpineItemsCount() - 1); +} // Calculate progress in book uint8_t Epub::calculateProgress(const int currentSpineIndex, const float currentSpineRead) { + size_t bookSize = getBookSize(); + if (bookSize == 0) { + return 0; + } size_t prevChapterSize = (currentSpineIndex >= 1) ? getCumulativeSpineItemSize(currentSpineIndex - 1) : 0; size_t curChapterSize = getCumulativeSpineItemSize(currentSpineIndex) - prevChapterSize; - size_t bookSize = getBookSize(); size_t sectionProgSize = currentSpineRead * curChapterSize; return round(static_cast(prevChapterSize + sectionProgSize) / bookSize * 100.0); } From c1d5f5d5629758f0b645f002775fc0c3d663cd82 Mon Sep 17 00:00:00 2001 From: IFAKA <99131130+IFAKA@users.noreply.github.com> Date: Fri, 19 Dec 2025 13:28:43 +0100 Subject: [PATCH 6/6] Add NULL checks after fopen() in ZipFile (#68) ## Problem Three `fopen()` calls in ZipFile.cpp did not check for NULL before using the file handle. If files cannot be opened, `fseek`/`fread`/`fclose` receive NULL and crash. ## Fix Added NULL checks with appropriate error logging and early returns for all three locations: - `getDataOffset()` - `readFileToMemory()` - `readFileToStream()` ## Testing - Builds successfully with `pio run` - Affects: `lib/ZipFile/ZipFile.cpp` --- lib/ZipFile/ZipFile.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/ZipFile/ZipFile.cpp b/lib/ZipFile/ZipFile.cpp index 30b44f8b..f55bb856 100644 --- a/lib/ZipFile/ZipFile.cpp +++ b/lib/ZipFile/ZipFile.cpp @@ -62,6 +62,10 @@ long ZipFile::getDataOffset(const mz_zip_archive_file_stat& fileStat) const { const uint64_t fileOffset = fileStat.m_local_header_ofs; FILE* file = fopen(filePath.c_str(), "r"); + if (!file) { + Serial.printf("[%lu] [ZIP] Failed to open file for reading local header\n", millis()); + return -1; + } fseek(file, fileOffset, SEEK_SET); const size_t read = fread(pLocalHeader, 1, localHeaderSize, file); fclose(file); @@ -104,6 +108,10 @@ uint8_t* ZipFile::readFileToMemory(const char* filename, size_t* size, const boo } FILE* file = fopen(filePath.c_str(), "rb"); + if (!file) { + Serial.printf("[%lu] [ZIP] Failed to open file for reading\n", millis()); + return nullptr; + } fseek(file, fileOffset, SEEK_SET); const auto deflatedDataSize = static_cast(fileStat.m_comp_size); @@ -175,6 +183,10 @@ bool ZipFile::readFileToStream(const char* filename, Print& out, const size_t ch } FILE* file = fopen(filePath.c_str(), "rb"); + if (!file) { + Serial.printf("[%lu] [ZIP] Failed to open file for streaming\n", millis()); + return false; + } fseek(file, fileOffset, SEEK_SET); const auto deflatedDataSize = static_cast(fileStat.m_comp_size);