From 631c63d5a4a46914a521d1a7adcf88caf6b99902 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 7 Feb 2022 14:56:13 +0100
Subject: [PATCH 1/2] term: scrollback-to-text: crash when trying to extract
 the entire scrollback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We calculated the ‘start’ row by adding the number of screen rows to
the current grid offset. This works in most cases. But not when the
offset is close to the wrap-around.

This triggered a crash when we tried to access a row number larger
than the number of available grid rows.

Fix by bounding the start row to the number of grid rows.

This unearthed a second bug, where trying to extract the scrollback
resulted in nothing getting copied.

The extraction logic did:

   for (r = start; r != (end + 1); r++)
       ....

This works, as long as end isn’t start-1. When we try to extract the
entire scrollback, it _is_ start-1.

Fix by rewriting the loop logic to check for r==end *after* copying
the row contents, but *before* incrementing r.

Closes #926
---
 terminal.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/terminal.c b/terminal.c
index 81df743b..631e8e68 100644
--- a/terminal.c
+++ b/terminal.c
@@ -3414,16 +3414,22 @@ rows_to_text(const struct terminal *term, int start, int end,
     if (ctx == NULL)
         return false;
 
-    for (size_t r = start;
-         r != ((end + 1) & (term->grid->num_rows - 1));
-         r = (r + 1) & (term->grid->num_rows - 1))
-    {
+    const int grid_rows = term->grid->num_rows;
+    int r = start;
+
+    while (true) {
         const struct row *row = term->grid->rows[r];
         xassert(row != NULL);
 
         for (int c = 0; c < term->cols; c++)
             if (!extract_one(term, row, &row->cells[c], c, ctx))
                 goto out;
+
+        if (r == end)
+            break;
+
+        r++;
+        r &= grid_rows - 1;
     }
 
 out:
@@ -3433,19 +3439,22 @@ out:
 bool
 term_scrollback_to_text(const struct terminal *term, char **text, size_t *len)
 {
-    int start = term->grid->offset + term->rows;
-    int end = term->grid->offset + term->rows - 1;
+    const int grid_rows = term->grid->num_rows;
+    int start = (term->grid->offset + term->rows) & (grid_rows - 1);
+    int end = (term->grid->offset + term->rows - 1) & (grid_rows - 1);
+
+    xassert(start >= 0);
+    xassert(start < grid_rows);
+    xassert(end >= 0);
+    xassert(end < grid_rows);
 
     /* If scrollback isn't full yet, this may be NULL, so scan forward
      * until we find the first non-NULL row */
     while (term->grid->rows[start] == NULL) {
         start++;
-        start &= term->grid->num_rows - 1;
+        start &= grid_rows - 1;
     }
 
-    if (end < 0)
-        end += term->grid->num_rows;
-
     while (term->grid->rows[end] == NULL) {
         end--;
         if (end < 0)

From 91be6d2e6ebbc977c27f72a6a7320c4aa7768b4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 7 Feb 2022 15:01:38 +0100
Subject: [PATCH 2/2] changelog: crash in pipe-scrollback

---
 CHANGELOG.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 309b8e99..66c5a49b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -57,6 +57,8 @@
   ongoing (https://codeberg.org/dnkl/foot/issues/922).
 * Large selections crossing the scrollback wrap-around
   (https://codeberg.org/dnkl/foot/issues/924).
+* Crash in `pipe-scrollback`
+  (https://codeberg.org/dnkl/foot/issues/926).
 
 
 ### Security