From 7bc22862fa9536cb2f18b030209a95306e57360d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Tue, 28 Mar 2023 18:31:24 +0200
Subject: [PATCH] render: protect against integer underflow when calculating
 scroll area
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When applying scroll damage, we calculate the affected region’s
height (in pixels), by subtracting the number of rows to scroll, from
the scrolling region, and finally multiply by the cell height.

If the number of rows to scroll is very large, the subtraction may
underflow, resulting in a very large height value instead of a
negative one.

This caused the check for "scrolling too many lines" to fail. That in
turn resulted in an integer overflow when calculating the source
offset into the rendered surface buffer, which typically triggered a
segfault.

This bug happened when there was continuous output in the terminal
without any new frames being rendered. This caused a buildup of scroll
damage, that triggered the underflow+overflow when we finally did
render a new frame.

For example, a compositor that doesn’t send any frame callbacks (for
example because the terminal window is minimized, or on a different
workspace/tag) would cause this.

Closes #1305
---
 CHANGELOG.md |  3 +++
 render.c     | 22 ++++++++++++++++------
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a02b9346..df098c41 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -136,6 +136,8 @@
   ([#1280][1280]).
 * Multi-character compose sequences with the kitty keyboard protocol
   ([#1288][1288]).
+* Crash when application output scrolls very fast, e.g. `yes`
+  ([#1305][1305]).
 
 [1173]: https://codeberg.org/dnkl/foot/issues/1173
 [1190]: https://codeberg.org/dnkl/foot/issues/1190
@@ -147,6 +149,7 @@
 [1249]: https://codeberg.org/dnkl/foot/issues/1249
 [1280]: https://codeberg.org/dnkl/foot/issues/1280
 [1288]: https://codeberg.org/dnkl/foot/issues/1288
+[1305]: https://codeberg.org/dnkl/foot/issues/1305
 
 
 ### Security
diff --git a/render.c b/render.c
index 2aba3237..f16898b4 100644
--- a/render.c
+++ b/render.c
@@ -929,14 +929,19 @@ static void
 grid_render_scroll(struct terminal *term, struct buffer *buf,
                    const struct damage *dmg)
 {
-    int height = (dmg->region.end - dmg->region.start - dmg->lines) * term->cell_height;
-
     LOG_DBG(
         "damage: SCROLL: %d-%d by %d lines",
         dmg->region.start, dmg->region.end, dmg->lines);
 
-    if (height <= 0)
+    const int region_size = dmg->region.end - dmg->region.start;
+
+    if (dmg->lines >= region_size) {
+        /* The entire scroll region will be scrolled out (i.e. replaced) */
         return;
+    }
+
+    const int height = (region_size - dmg->lines) * term->cell_height;
+    xassert(height > 0);
 
 #if TIME_SCROLL_DAMAGE
     struct timespec start_time;
@@ -1037,14 +1042,19 @@ static void
 grid_render_scroll_reverse(struct terminal *term, struct buffer *buf,
                            const struct damage *dmg)
 {
-    int height = (dmg->region.end - dmg->region.start - dmg->lines) * term->cell_height;
-
     LOG_DBG(
         "damage: SCROLL REVERSE: %d-%d by %d lines",
         dmg->region.start, dmg->region.end, dmg->lines);
 
-    if (height <= 0)
+    const int region_size = dmg->region.end - dmg->region.start;
+
+    if (dmg->lines >= region_size) {
+        /* The entire scroll region will be scrolled out (i.e. replaced) */
         return;
+    }
+
+    const int height = (region_size - dmg->lines) * term->cell_height;
+    xassert(height > 0);
 
 #if TIME_SCROLL_DAMAGE
     struct timespec start_time;