Bound BMP rows to the row buffer and show failed sends

tx_bmp_open() accepted 24- and 32-bit BMPs of any width, then each source
row was read into uint8_t row_buf[128] on the stack. A 24-bit BMP wider
than 42 px, or a 1-bit BMP wider than 1024 px, overran it. The decoder
also reads one bit per pixel, so 24/32-bit files were misread anyway.
A host build of the previous code under AddressSanitizer reports a
132-byte write into the 128-byte buffer for a 24-bit, 43 px wide file.

- Accept only 1bpp and the stacked-plane "2bpp" layout, reject zero or
  over-16-bit sizes, and reject rows wider than TX_BMP_ROW_BUF_SIZE.
- Read header fields as uint32_t. Shifting into bit 31 of an int and
  negating INT32_MIN were undefined.
- The result screen showed "Flipped ;)" and played the success sound
  even when the send failed or was cancelled. Show "Send failed" with
  the error sound, or "Stopped" with no sound when cancelled with Back.
- Check for a stop request after the gap between frames of a sequence.
  tagtinker_ir_transmit() clears the stop flag on entry, so a Back
  pressed during the gap was lost and the next frame went out in full.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Hoggormino
2026-09-16 17:10:50 +02:00
co-authored by Claude Opus 5
parent b6ebe46d0f
commit 692e436fe5
+53 -22
View File
@@ -15,6 +15,7 @@ typedef struct {
uint32_t tick;
bool completed;
bool ok;
bool cancelled;
} TxViewModel;
typedef struct {
@@ -36,6 +37,10 @@ typedef struct {
#define TX_COLOR26_PACKED_BYTES \
(((size_t)TAGTINKER_COLOR26_GLASS_W * (size_t)TAGTINKER_COLOR26_GLASS_H + 7U) / 8U)
/* One BMP source row is read into a stack buffer of this size. 128 bytes is
* a 1024 px wide 1bpp row; the widest profile is 800 px. */
#define TX_BMP_ROW_BUF_SIZE 128U
static uint16_t tx_pick_chunk_height(uint16_t width, uint16_t height, bool second_plane);
static void tx_debug_log(const char* fmt, ...) {
@@ -549,6 +554,11 @@ static bool tx_stream_text_image(TagTinkerApp* app) {
return ok;
}
static uint32_t tx_le32(const uint8_t* p) {
return (uint32_t)p[0] | ((uint32_t)p[1] << 8) | ((uint32_t)p[2] << 16) |
((uint32_t)p[3] << 24);
}
static bool tx_bmp_open(const char* path, File* file, TxBmpInfo* info) {
if(!storage_file_open(file, path, FSAM_READ, FSOM_OPEN_EXISTING)) return false;
@@ -557,26 +567,28 @@ static bool tx_bmp_open(const char* path, File* file, TxBmpInfo* info) {
if(header[0] != 'B' || header[1] != 'M') return false;
uint16_t bpp = header[28] | (header[29] << 8);
if(!(bpp == 1 || bpp == 2 || bpp == 24 || bpp == 32)) return false;
/* The row decoder reads one bit per pixel. "2bpp" here means two stacked
* 1-bit planes, as written by the web image preparer and the WiFi plugin
* path. 24/32-bit files would be misread, and their rows do not fit the
* row buffer, so they are rejected. */
if(!(bpp == 1 || bpp == 2)) return false;
info->bpp = bpp;
int32_t bmp_h = header[22] | (header[23] << 8) | (header[24] << 16) | (header[25] << 24);
info->width = (uint16_t)(header[18] | (header[19] << 8) | (header[20] << 16) | (header[21] << 24));
info->top_down = false;
if(bmp_h < 0) {
info->top_down = true;
bmp_h = -bmp_h;
}
/* Read the 32-bit fields unsigned: shifting a byte into bit 31 of an int
* is undefined, and so is negating INT32_MIN. A negative height means a
* top-down BMP. Sizes that do not fit the 16-bit fields are rejected
* rather than silently truncated. */
uint32_t raw_w = tx_le32(&header[18]);
uint32_t raw_h = tx_le32(&header[22]);
info->top_down = (raw_h & 0x80000000U) != 0U;
uint32_t abs_h = info->top_down ? (~raw_h + 1U) : raw_h;
if(raw_w == 0U || raw_w > UINT16_MAX || abs_h == 0U || abs_h > UINT16_MAX) return false;
info->height = (uint16_t)bmp_h;
info->data_offset = header[10] | (header[11] << 8) | (header[12] << 16) | (header[13] << 24);
if(info->bpp == 1 || info->bpp == 2) {
info->row_stride = ((info->width + 31U) / 32U) * 4U;
} else if(info->bpp == 24) {
info->row_stride = ((info->width * 3U) + 3U) & ~3U;
} else {
info->row_stride = info->width * 4U;
}
info->width = (uint16_t)raw_w;
info->height = (uint16_t)abs_h;
info->data_offset = tx_le32(&header[10]);
info->row_stride = ((info->width + 31U) / 32U) * 4U;
if(info->row_stride > TX_BMP_ROW_BUF_SIZE) return false;
return true;
}
@@ -798,9 +810,8 @@ static bool tx_stream_bmp_image(TagTinkerApp* app) {
uint16_t plane2_off_rows = info.height;
UNUSED(accent_color);
/* Source row stride is bounded by max profile width (800 px) -> 104 B,
* round up generously to 128 to absorb any future profile additions. */
uint8_t row_buf[128];
/* tx_bmp_open() rejects any file whose row stride exceeds this buffer. */
uint8_t row_buf[TX_BMP_ROW_BUF_SIZE];
uint16_t cached_src_y = UINT16_MAX;
/* ---- PASS 1: Count RLE compressed bit length ---- */
@@ -996,8 +1007,14 @@ static int32_t tx_thread_callback(void* context) {
ok = tx_stream_bmp_image(app);
} else if(app->frame_seq_count > 0) {
for(size_t i = 0; i < app->frame_seq_count; i++) {
if(!app->tx_active) { ok = false; break; }
if(i > 0) furi_delay_ms(20);
/* Check after the gap: tagtinker_ir_transmit() clears a stop
* request on entry, so a Back pressed during the gap would
* otherwise be lost and the next frame sent in full. */
if(!app->tx_active) {
ok = false;
break;
}
ok = tagtinker_ir_transmit(
app->frame_sequence[i], app->frame_lengths[i],
tx_apply_signal_mode(app, app->frame_repeats[i]), 10);
@@ -1051,6 +1068,10 @@ static void transmit_draw_cb(Canvas* canvas, void* _model) {
canvas_draw_circle(canvas, x, dot_y, 1);
}
}
} else if(model->cancelled) {
canvas_draw_str_aligned(canvas, 64, 30, AlignCenter, AlignTop, "Stopped");
} else if(model->completed && !model->ok) {
canvas_draw_str_aligned(canvas, 64, 30, AlignCenter, AlignTop, "Send failed");
} else {
canvas_draw_str_aligned(canvas, 64, 30, AlignCenter, AlignTop, "Flipped ;)");
}
@@ -1080,6 +1101,7 @@ void tagtinker_scene_transmit_on_enter(void* context) {
model->tick = 0;
model->completed = false;
model->ok = true;
model->cancelled = false;
view_commit_model(app->transmit_view, true);
view_dispatcher_switch_to_view(app->view_dispatcher, TagTinkerViewTransmit);
@@ -1096,6 +1118,9 @@ bool tagtinker_scene_transmit_on_event(void* context, SceneManagerEvent event) {
if(app->tx_active) {
app->tx_active = false;
tagtinker_ir_stop();
TxViewModel* model = view_get_model(app->transmit_view);
model->cancelled = true;
view_commit_model(app->transmit_view, true);
return true;
} else {
if(!scene_manager_search_and_switch_to_previous_scene(app->scene_manager, TagTinkerSceneTargetActions)) {
@@ -1121,8 +1146,14 @@ bool tagtinker_scene_transmit_on_event(void* context, SceneManagerEvent event) {
TxViewModel* model = view_get_model(app->transmit_view);
model->completed = true;
model->ok = (event.event == 101);
bool cancelled = model->cancelled;
bool ok = model->ok;
view_commit_model(app->transmit_view, true);
notification_message(app->notifications, &sequence_success);
/* A cancelled send gets no sound: its last frame may have gone out
* fine, but the user stopped it. */
if(!cancelled) {
notification_message(app->notifications, ok ? &sequence_success : &sequence_error);
}
}
return true;
}