unexpand: fix heap buffer overflow with --tabs=[+/]NUM

This avoids CWE-122: Heap-based Buffer Overflow
where we could write blank characters beyond
the allocated heap buffer.

* src/expand-common.c (set_max_column_width): Refactor function from ...
(add_tab_stop): ... here.
(set_extend_size): Call new function.
(set_increment_size): Likewise.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/79555

diff --git a/NEWS b/NEWS
index a19e3aed602207a112a9537c8b390f00fbdc5702..b8c4ed4ef03595443ca9d1b709aa1c5d22b36880 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   Previously it may have output too few lines.
   [bug introduced in coreutils-9.8]
 
+  'unexpand' no longer triggers a heap buffer overflow with --tabs arguments
+  that use the GNU extension /NUM or +NUM formats.
+  [bug introduced in coreutils-8.28]
+
 ** Improvements
 
   wc -l now operates 10% faster on hosts that support AVX512 instructions.

diff --git a/src/expand-common.c b/src/expand-common.c
index ca2ad4d67b60e6401fce5bcf4b186a57e9a3f8b6..14dd804f91f887ea62882057781e95644dfbaa66 100644 (file)
--- a/src/expand-common.c
+++ b/src/expand-common.c
@@ -70,6 +70,15 @@ static bool have_read_stdin = false;
 int exit_status = EXIT_SUCCESS;
 
 
+static void
+set_max_column_width (colno width)
+{
+  if (max_column_width < width)
+    {
+      if (ckd_add (&max_column_width, width, 0))
+        error (EXIT_FAILURE, 0, _("tabs are too far apart"));
+    }
+}
 
 /* Add tab stop TABVAL to the end of 'tab_list'.  */
 extern void
@@ -82,11 +91,7 @@ add_tab_stop (colno tabval)
     tab_list = xpalloc (tab_list, &n_tabs_allocated, 1, -1, sizeof *tab_list);
   tab_list[first_free_tab++] = tabval;
 
-  if (max_column_width < column_width)
-    {
-      if (ckd_add (&max_column_width, column_width, 0))
-        error (EXIT_FAILURE, 0, _("tabs are too far apart"));
-    }
+  set_max_column_width (column_width);
 }
 
 static bool
@@ -103,6 +108,8 @@ set_extend_size (colno tabval)
     }
   extend_size = tabval;
 
+  set_max_column_width (extend_size);
+
   return ok;
 }
 
@@ -120,6 +127,8 @@ set_increment_size (colno tabval)
     }
   increment_size = tabval;
 
+  set_max_column_width (increment_size);
+
   return ok;
 }
 

diff --git a/tests/misc/unexpand.pl b/tests/misc/unexpand.pl
index 27d9c17b6a83f479b46244172f0922a0308c4314..bb7469cae351d54021a01052029e8f51aea16684 100755 (executable)
--- a/tests/misc/unexpand.pl
+++ b/tests/misc/unexpand.pl
@@ -76,6 +76,10 @@ my @Tests =
      ['blanks-12', '-t', '3,4', {IN=> "01  4\n"}, {OUT=> "01\t\t4\n"}],
      ['blanks-13', '-t', '3,4', {IN=> "0   4\n"}, {OUT=> "0\t\t4\n"}],
 
+     # These would overflow a heap buffer from v8.28 - v9.8 inclusive
+     ['blanks-ext1', '-t', '3,+6', {IN=> "\t      "}, {OUT=> "\t\t"}],
+     ['blanks-ext2', '-t', '3,/9', {IN=> "\t      "}, {OUT=> "\t\t"}],
+
      # POSIX says spaces should only follow tabs. Also a single
      # trailing space is not converted to a tab, when before
      # a field starting with non blanks