GNU bug report logs - #11787
Potential use after free bug in coreutils 8.17

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11787 in the body.
You can then email your comments to 11787 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Xu Zhongxing" <xu_zhong_xing <at> 163.com>
To: bug-coreutils <at> gnu.org
Subject: Potential use after free bug in coreutils 8.17
Date: Tue, 26 Jun 2012 13:01:13 +0800 (CST)

[Message part 1 (text/plain, inline)]

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

[Message part 2 (text/html, inline)]

Message #10 received at 11787-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Xu Zhongxing <xu_zhong_xing <at> 163.com>
Cc: 11787-done <at> debbugs.gnu.org
Subject: Re: bug#11787: Potential use after free bug in coreutils 8.17
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 <P <at> draigBrady.com>
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                        wis.macomson <at> intel.com
 Wojciech Purczynski                 cliph <at> isec.pl
 Wolfram Kleff                       kleff <at> cs.uni-bonn.de
 Won-kyu Park                        wkpark <at> chem.skku.ac.kr
+Xu Zhongxing                        xu_zhong_xing <at> 163.com
 Yang Ren                            ryang <at> redhat.com
 Yanko Kaneti                        yaneti <at> declera.com
 Yann Dirson                         dirson <at> debian.org
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;
 }

Message #11 received at 11787-done <at> debbugs.gnu.org (full text, mbox):

From: Xu Zhongxing <xu_zhong_xing <at> 163.com>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 11787-done <at> debbugs.gnu.org
Subject: Re: bug#11787: Potential use after free bug in coreutils 8.17
Date: Tue, 26 Jun 2012 20:33:09 +0800

I would like to mention that this bug (and a previous one) was found by 
a static analysis tool called Canalyze developed by us. See 
http://lcs.ios.ac.cn/~xzx/ <http://lcs.ios.ac.cn/%7Exzx/>

于 2012/6/26 18:32, Pádraig Brady 写道:
> 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<P <at> draigBrady.com>
> 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                        wis.macomson <at> intel.com
>   Wojciech Purczynski                 cliph <at> isec.pl
>   Wolfram Kleff                       kleff <at> cs.uni-bonn.de
>   Won-kyu Park                        wkpark <at> chem.skku.ac.kr
> +Xu Zhongxing                        xu_zhong_xing <at> 163.com
>   Yang Ren                            ryang <at> redhat.com
>   Yanko Kaneti                        yaneti <at> declera.com
>   Yann Dirson                         dirson <at> debian.org
> 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;
>   }

Message #12 received at 11787-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Xu Zhongxing <xu_zhong_xing <at> 163.com>
Cc: 11787-done <at> debbugs.gnu.org
Subject: Re: bug#11787: Potential use after free bug in coreutils 8.17
Date: Tue, 26 Jun 2012 22:09:42 +0100

On 06/26/2012 01:33 PM, Xu Zhongxing wrote:
> I would like to mention that this bug (and a previous one) was found by a static analysis tool called Canalyze developed by us.

I guessed that as you can see from the commit message below :)

> See http://lcs.ios.ac.cn/~xzx/

I'll include that URL since you imply it's fairly permanent.

cheers,
Pádraig.

> 
> 于 2012/6/26 18:32, Pádraig Brady 写道:
>> 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<P <at> draigBrady.com>
>> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.