render: protect against integer underflow when calculating scroll area

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
This commit is contained in:
Daniel Eklöf 2023-03-28 18:31:24 +02:00
parent 3215d54f31
commit 7bc22862fa
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
2 changed files with 19 additions and 6 deletions

View file

@ -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

View file

@ -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;