pr: fix read from invalid memory with tabs in separator

pixelb · pixelb · commit d91aeef0527b · 2016-11-25T14:08:42.000Z
This was detected with: echo a > a; pr "-S$(printf "\t\t\t")" a -m a > /dev/null Resulting in ASAN triggering: ==================================================== ERROR: AddressSanitizer: global-buffer-overflow READ of size 1 at 0x00000041b622 thread T0 #0 0x40506a in print_sep_string ../src/pr.c:2241 #1 0x407ec4 in read_line ../src/pr.c:2493 #2 0x40985c in print_page ../src/pr.c:1802 #3 0x40985c in print_files ../src/pr.c:1618 #4 0x4036e0 in main ../src/pr.c:1136 * src/pr.c (init_parameters): Ensure we only override the specified separator when it's a single tab, thus matching the calculated separator length. * tests/pr/pr-tests.pl: Add a test case. * NEWS: Mention the fix.
diff --git a/NEWS b/NEWS
@@ -35,6 +35,10 @@ GNU coreutils NEWS                                    -*- outline -*-
   nl now resets numbering for each page section rather than just for each page.
   [This bug was present in "the beginning".]
 
+  pr now handles specified separator strings containing tabs correctly.
+  Previously it would have output random data from memory.
+  [This bug was detected with ASAN and present in "the beginning".]
+
   sort -h -k now works even in locales that use blank as thousands separator.
 
   stty --help no longer outputs extraneous gettext header lines
diff --git a/src/pr.c b/src/pr.c
@@ -1233,7 +1233,7 @@ init_parameters (int number_of_files)
         }
       /* It's rather pointless to define a TAB separator with column
          alignment */
-      else if (!join_lines && *col_sep_string == '\t')
+      else if (!join_lines && col_sep_length == 1 && *col_sep_string == '\t')
         col_sep_string = column_separator;
 
       truncate_lines = true;
diff --git a/tests/pr/pr-tests.pl b/tests/pr/pr-tests.pl
@@ -467,6 +467,13 @@
     {IN=>{3=>"x\ty\tz\n"}},
      {OUT=>join("\t", qw(a b c m n o x y z)) . "\n"} ];
 
+# This resulted in reading invalid memory before coreutils-8.26
+push @Tests,
+   ['asan1', "-m -S'\t\t\t' -t",
+    {IN=>{1=>"a\n"}},
+    {IN=>{2=>"a\n"}},
+     {OUT=>"a\t\t\t\t  \t\t\ta\n"} ];
+
 @Tests = triple_test \@Tests;
 
 my $save_temps = $ENV{DEBUG};

Original file line number	Diff line number	Diff line change
`@@ -1233,7 +1233,7 @@ init_parameters (int number_of_files)`
`1233`	`1233`	`}`
`1234`	`1234`	`/* It's rather pointless to define a TAB separator with column`
`1235`	`1235`	`alignment */`
`1236`		`- else if (!join_lines && *col_sep_string == '\t')`
	`1236`	`+ else if (!join_lines && col_sep_length == 1 && *col_sep_string == '\t')`
`1237`	`1237`	`col_sep_string = column_separator;`
`1238`	`1238`
`1239`	`1239`	`truncate_lines = true;`