From 981e4b77cb96ab277ea3b7a1e8aebf0e4589c032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Tue, 28 Mar 2023 18:37:41 +0200 Subject: [PATCH] term: protect against integer overflow when accumulating scroll damage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When accumulating scroll damage, we check if the last scroll damage’s scrolling region, and type, matches the new/current scroll damage. If so, the number of lines in the last scroll damage is increased, instead of adding a new scroll damage instance to the list. If the scroll damage list isn’t consumed, this build up of scroll damage would eventually overflow. And, even if it didn’t overflow, it could become large enough, that when later used to calculate e.g. the affected surface area, while rendering a frame, would cause an overflow there instead. This patch fixes both issues by: a) do an overflow check before increasing the line count b) limit the line count to UINT16_MAX --- CHANGELOG.md | 1 + terminal.c | 17 +++++++++++------ terminal.h | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df098c41..30a34f01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -138,6 +138,7 @@ ([#1288][1288]). * Crash when application output scrolls very fast, e.g. `yes` ([#1305][1305]). +* Crash when application scrolls **many** lines (> ~2³¹). [1173]: https://codeberg.org/dnkl/foot/issues/1173 [1190]: https://codeberg.org/dnkl/foot/issues/1190 diff --git a/terminal.c b/terminal.c index 78ce4bc3..04153513 100644 --- a/terminal.c +++ b/terminal.c @@ -2252,15 +2252,20 @@ void term_damage_scroll(struct terminal *term, enum damage_type damage_type, struct scroll_region region, int lines) { - if (tll_length(term->grid->scroll_damage) > 0) { + if (likely(tll_length(term->grid->scroll_damage) > 0)) { struct damage *dmg = &tll_back(term->grid->scroll_damage); - if (dmg->type == damage_type && - dmg->region.start == region.start && - dmg->region.end == region.end) + if (likely( + dmg->type == damage_type && + dmg->region.start == region.start && + dmg->region.end == region.end)) { - dmg->lines += lines; - return; + /* Make sure we don’t overflow... */ + int new_line_count = (int)dmg->lines + lines; + if (likely(new_line_count <= UINT16_MAX)) { + dmg->lines = new_line_count; + return; + } } } struct damage dmg = { diff --git a/terminal.h b/terminal.h index 21236797..d2762a5a 100644 --- a/terminal.h +++ b/terminal.h @@ -96,7 +96,7 @@ enum damage_type {DAMAGE_SCROLL, DAMAGE_SCROLL_REVERSE, struct damage { enum damage_type type; struct scroll_region region; - int lines; + uint16_t lines; }; struct row_uri_range {