Subject: Re: bug#11787: Potential use after free bug in coreutils 8.17

From: Pádraig Brady <[email-masked]>
Date: Tue, 26 Jun 2012 11:32:56 +0100

On 06/26/2012 06:01 AM, Xu Zhongxing wrote:
> In Coreutils 8.17, csplit.c, static bool load_buffer (void)
> 
> On line 503 and 511, b is passed to free_buffer() twice. This could lead to a use-after-free bug in free_buffer(): struct line *n = l->next;, where buf->line_start is freed in the first call of free_buffer().
> 
> - Xu Zhongxing

I think this will address it.

thanks!
Pádraig.

commit 5958bb44c4d7cf3b69bb62955b3ece9d0715eb60
Author: Pádraig Brady <[email-masked]>
Date:   Tue Jun 26 11:13:45 2012 +0100

    maint: avoid a static analysis warning in csplit

    The Canalyze static code analyzer correctly surmised
    that there is a use-after-free bug in free_buffer()
    at the line "struct line *n = l->next", if that
    function is called multiple times.

    This is not a runtime issue since a list of lines
    will not be present in the !lines_found case.

    * src/csplit.c (free_buffer): Set list head to NULL so
    that this function can be called multiple times.
    (load_buffer): Remove a redundant call to free_buffer().

    Reported-by: Xu Zhongxing

diff --git a/THANKS.in b/THANKS.in
index 51b2c7d..2bdeab5 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -636,6 +636,7 @@ Wis Macomson                        [email-masked]
 Wojciech Purczynski                 [email-masked]
 Wolfram Kleff                       [email-masked]
 Won-kyu Park                        [email-masked]
+Xu Zhongxing                        [email-masked]
 Yang Ren                            [email-masked]
 Yanko Kaneti                        [email-masked]
 Yann Dirson                         [email-masked]
diff --git a/src/csplit.c b/src/csplit.c
index fb43350..c10562b 100644
--- a/src/csplit.c
+++ b/src/csplit.c
@@ -425,6 +425,7 @@ free_buffer (struct buffer_record *buf)
       free (l);
       l = n;
     }
+  buf->line_start = NULL;
   free (buf->buffer);
   buf->buffer = NULL;
 }
@@ -499,8 +500,6 @@ load_buffer (void)
       b->bytes_used += read_input (p, bytes_avail);

       lines_found = record_line_starts (b);
-      if (!lines_found)
-        free_buffer (b);

       if (lines_found || have_read_eof)
         break;
@@ -515,7 +514,10 @@ load_buffer (void)
   if (lines_found)
     save_buffer (b);
   else
-    free (b);
+    {
+      free_buffer (b);
+      free (b);
+    }

   return lines_found != 0;
 }