tac: avoid double free

* src/tac.c (main): Reading a line longer than 16KiB would cause
tac to realloc its primary buffer.  Then, just before exit, tac
would mistakenly free the original (now free'd) buffer.
This bug was introduced by commit be6c13e7, "maint: always free a
buffer, to avoid even semblance of a leak".
* NEWS (Bug fixes): Mention it.
* tests/misc/tac (double-free): New test, to exercise this.
Reported by Salvo Tomaselli in <http://bugs.debian.org/594666>.

diff --git a/NEWS b/NEWS
index 93a1f96b4cf301c671864f093c52f7459890762f..3eb28b1e3beb4d130bfa23f2951799ff96a42fd3 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   du -H and -L now consistently count pointed-to files instead of
   symbolic links, and correctly diagnose dangling symlinks.
 
+  tac would perform a double-free when given an input line longer than 16KiB.
+  [bug introduced in coreutils-8.3]
+
 ** New features
 
   cp now accepts the --attributes-only option to not copy file data,

diff --git a/src/tac.c b/src/tac.c
index cec97360191091f3eefb5f549bf52c395466d5ac..859e0067a4db64f805d4acfb8a8f65a26bb77835 100644 (file)
--- a/src/tac.c
+++ b/src/tac.c
@@ -633,7 +633,6 @@ main (int argc, char **argv)
   if (! (read_size < half_buffer_size && half_buffer_size < G_buffer_size))
     xalloc_die ();
   G_buffer = xmalloc (G_buffer_size);
-  void *buf = G_buffer;
   if (sentinel_length)
     {
       strcpy (G_buffer, separator);
@@ -666,6 +665,9 @@ main (int argc, char **argv)
       error (0, errno, "-");
       ok = false;
     }
-  free (buf);
+
+  size_t offset = sentinel_length ? sentinel_length : 1;
+  free (G_buffer - offset);
+
   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 }

diff --git a/tests/misc/tac b/tests/misc/tac
index 76310499338c5fca375a1bd67f1d1bc39c1df580..4130c004d924f830f9b74312853ba72d8d216332 100755 (executable)
--- a/tests/misc/tac
+++ b/tests/misc/tac
@@ -24,6 +24,9 @@ my $prog = 'tac';
 
 my $bad_dir = 'no/such/dir';
 
+# This must be longer than 16KiB to trigger the double free in coreutils-8.5.
+my $long_line = 'o' x (16 * 1024 + 1);
+
 my @Tests =
 (
   ['segfault', '-r', {IN=>"a\n"}, {IN=>"b\n"}, {OUT=>"a\nb\n"}],
@@ -67,6 +70,9 @@ my @Tests =
    {ERR_SUBST => "s,`$bad_dir': .*,...,"},
    {ERR => "$prog: cannot create temporary file in ...\n"},
    {EXIT => 1}],
+
+  # coreutils-8.5's tac would double-free its primary buffer.
+  ['double-free', {IN=>$long_line}, {OUT=>$long_line}],
 );
 
 @Tests = triple_test \@Tests;