GNU bug report logs - #16171
ptx: heap buffer overrun, when run with two file arguments

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 16171 in the body.
You can then email your comments to 16171 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: Jim Meyering <jim <at> meyering.net>
To: bug-coreutils <at> gnu.org
Subject: ptx: heap buffer overrun, when run with two file arguments
Date: Mon, 16 Dec 2013 18:22:27 -0800

Hi,

I built like this using just-built 4.9.0 20131216
(but it probably would work as well with 4.8.x):

  make check AM_CFLAGS='-ggdb3 -static-libasan -fsanitize=address'
         AM_LDFLAGS='-fsanitize=address -static-libasan -lpthread -ldl'

and then I ran this,

  echo a > a && echo b > b &&
  ./ptx -g1 -w1 a b 2>&1 | asan_symbolize.py -d

and include its output below.
That output shows a heap-read overrun bug that arises
because ptx was designed to process only one input file, yet
was later extended to process more than, but without some
important adjustments.

The underlying problem is that swallow_file_in_memory (called from main)
is setting the contents of the global text_buffer for the first file,
then updating it (clobbering old value) for the second file.
Yet, some pointers to the initial buffer have been squirreled away
and later, one of them (keyafter) is presumed to point into
the new "text_buffer", which it does not.  The subsequent
SKIP_WHITE_BACKWARDS use backs up "cursor" until it is goes
out of bounds.

=================================================================
==3156==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x60200000e90f at pc 0x409612 bp 0x7fff67a426c0 sp 0x7fff67a426b8
READ of size 1 at 0x60200000e90f thread T0
    #0 0x409611 in generate_all_output /cu/src/ptx.c:1425
    #1 0x7f3080a89994 in __libc_start_main
/home/aurel32/eglibc/eglibc-2.17/csu/libc-start.c:276
    #2 0x409bb4 in _start ??:?

0x60200000e90f is located 1 bytes to the left of 3-byte region
[0x60200000e910,0x60200000e913)
allocated by thread T0 here:
    #0 0x4458e7 in __interceptor_malloc _asan_rtl_
    #1 0x462c8b in fread_file /cu/lib/read-file.c:73

SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
  0x0c047fff9cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d10: fa fa fa fa fa fa fd fd fa fa 03 fa fa fa fd fd
=>0x0c047fff9d20: fa[fa]03 fa fa fa 00 00 fa fa 04 fa fa fa 04 fa
  0x0c047fff9d30: fa fa fd fa fa fa fd fa fa fa 00 fa fa fa fd fa
  0x0c047fff9d40: fa fa 00 fa fa fa 00 fa fa fa fd fa fa fa fd fa
  0x0c047fff9d50: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
  0x0c047fff9d60: fa fa fd fa fa fa fd fa fa fa fd fa fa fa 00 fa
  0x0c047fff9d70: fa fa 00 fa fa fa 00 fa fa fa 00 fa fa fa 04 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==3156==ABORTING

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 16171 <at> debbugs.gnu.org
Subject: Re: bug#16171: ptx: heap buffer overrun,
 when run with two file arguments
Date: Tue, 17 Dec 2013 10:29:28 +0000

On 12/17/2013 02:22 AM, Jim Meyering wrote:
> Hi,
> 
> I built like this using just-built 4.9.0 20131216
> (but it probably would work as well with 4.8.x):
> 
>   make check AM_CFLAGS='-ggdb3 -static-libasan -fsanitize=address'
>          AM_LDFLAGS='-fsanitize=address -static-libasan -lpthread -ldl'
> 
> and then I ran this,
> 
>   echo a > a && echo b > b &&
>   ./ptx -g1 -w1 a b 2>&1 | asan_symbolize.py -d
> 
> and include its output below.
> That output shows a heap-read overrun bug that arises
> because ptx was designed to process only one input file, yet
> was later extended to process more than, but without some
> important adjustments.
> 
> The underlying problem is that swallow_file_in_memory (called from main)
> is setting the contents of the global text_buffer for the first file,
> then updating it (clobbering old value) for the second file.
> Yet, some pointers to the initial buffer have been squirreled away
> and later, one of them (keyafter) is presumed to point into
> the new "text_buffer", which it does not.  The subsequent
> SKIP_WHITE_BACKWARDS use backs up "cursor" until it is goes
> out of bounds.

Nice. This is a good illustration how test coverage
can be leveraged by (future) run time checks.

I see it here too (as the only failure in make check with -fsanitize=address

$ rpm -q gcc
gcc-4.8.2-1.fc20.x86_64
$ yum install libasan  # http://bugzilla.redhat.com/991003
$ rm src/ptx.o
$ make check AM_CFLAGS='-fsanitize=address' TESTS=tests/misc/ptx.pl SUBDIRS=. VERBOSE=yes
$ failure identified in tests/test-suite.log ...
$ src/ptx -g1 -w1 <(echo a) <(echo b) | asan_symbolize.py -d

thanks!
Pádraig.

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 16171-done <at> debbugs.gnu.org
Subject: Re: bug#16171: ptx: heap buffer overrun,
 when run with two file arguments
Date: Mon, 28 Apr 2014 14:52:23 +0100

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

On 12/17/2013 10:29 AM, Pádraig Brady wrote:
> On 12/17/2013 02:22 AM, Jim Meyering wrote:
>> Hi,
>>
>> I built like this using just-built 4.9.0 20131216
>> (but it probably would work as well with 4.8.x):
>>
>>   make check AM_CFLAGS='-ggdb3 -static-libasan -fsanitize=address'
>>          AM_LDFLAGS='-fsanitize=address -static-libasan -lpthread -ldl'
>>
>> and then I ran this,
>>
>>   echo a > a && echo b > b &&
>>   ./ptx -g1 -w1 a b 2>&1 | asan_symbolize.py -d
>>
>> and include its output below.
>> That output shows a heap-read overrun bug that arises
>> because ptx was designed to process only one input file, yet
>> was later extended to process more than, but without some
>> important adjustments.
>>
>> The underlying problem is that swallow_file_in_memory (called from main)
>> is setting the contents of the global text_buffer for the first file,
>> then updating it (clobbering old value) for the second file.
>> Yet, some pointers to the initial buffer have been squirreled away
>> and later, one of them (keyafter) is presumed to point into
>> the new "text_buffer", which it does not.  The subsequent
>> SKIP_WHITE_BACKWARDS use backs up "cursor" until it is goes
>> out of bounds.
> 
> Nice. This is a good illustration how test coverage
> can be leveraged by (future) run time checks.
> 
> I see it here too (as the only failure in make check with -fsanitize=address

The attached should address this.
I'll push later.

thanks,
Pádraig.

[ptx-whitespace-heap-overflow.patch (text/x-patch, attachment)]

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

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: 16171 <at> debbugs.gnu.org, P <at> draigBrady.com, jim <at> meyering.net
Subject: Re: bug#16171: ptx: heap buffer overrun,
 when run with two file arguments
Date: Mon, 28 Apr 2014 23:01:06 +0200

On 04/28/2014 03:52 PM, Pádraig Brady wrote:
> diff --git a/tests/misc/ptx-overrun.sh b/tests/misc/ptx-overrun.sh

> +# Trigger an invalid heap reference noticed by gcc -fsanitize=address
> +# from coreutils-8.22 and earlier.  As well as an invalid memory reference,
> +# the issue can be seen in the output, with invalid whitespace trimming
> +# when multiple files are specified.
> +printf '%s\n' 'This is a ptx whitespace Trimming test' > ws.in
> +ptx ws.in ws.in | sort | uniq -u > out
> +compare /dev/null out || fail=1

Isn't this a user-visible change, i.e., worth a NEWS entry?
+1 otherwise.


BTW: I noticed that v8.21 produces a different result than v8.22:

  $ /tmp/cu/coreutils-8.21/src/ptx ws.in ws.in | sort | uniq -u
      test                               This is a ptx whitespace Trimming
     test                                This is a ptx whitespace Trimming
  $ /tmp/cu/coreutils-8.22/src/ptx ws.in ws.in | sort | uniq -u
          is a ptx whitespace Trimming   test                            This
          is a ptx whitespace Trimming   test                           This

... although there hasn't been a change in src/ptx.c - or I don't see it:
"git diff -r v8.21..v8.22 src/ptx.c" only shows a change in usage().
Any idea why?

Have a nice day,
Berny

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>
Cc: 16171 <at> debbugs.gnu.org, jim <at> meyering.net
Subject: Re: bug#16171: ptx: heap buffer overrun,
 when run with two file arguments
Date: Mon, 28 Apr 2014 22:36:20 +0100

On 04/28/2014 10:01 PM, Bernhard Voelker wrote:
> On 04/28/2014 03:52 PM, Pádraig Brady wrote:
>> diff --git a/tests/misc/ptx-overrun.sh b/tests/misc/ptx-overrun.sh
> 
>> +# Trigger an invalid heap reference noticed by gcc -fsanitize=address
>> +# from coreutils-8.22 and earlier.  As well as an invalid memory reference,
>> +# the issue can be seen in the output, with invalid whitespace trimming
>> +# when multiple files are specified.
>> +printf '%s\n' 'This is a ptx whitespace Trimming test' > ws.in
>> +ptx ws.in ws.in | sort | uniq -u > out
>> +compare /dev/null out || fail=1
> 
> Isn't this a user-visible change, i.e., worth a NEWS entry?

Good point. I'll add a NEWS entry.

> BTW: I noticed that v8.21 produces a different result than v8.22:
> 
>   $ /tmp/cu/coreutils-8.21/src/ptx ws.in ws.in | sort | uniq -u
>       test                               This is a ptx whitespace Trimming
>      test                                This is a ptx whitespace Trimming
>   $ /tmp/cu/coreutils-8.22/src/ptx ws.in ws.in | sort | uniq -u
>           is a ptx whitespace Trimming   test                            This
>           is a ptx whitespace Trimming   test                           This

It's basically undefined behavior when trimming whitespace
depending on values on the heap. So if you look at the full output
it should be largely the same apart from the whitespace.

cheers,
Pádraig.

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

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigbrady.com>
Cc: 16171-done <at> debbugs.gnu.org
Subject: Re: bug#16171: ptx: heap buffer overrun,
 when run with two file arguments
Date: Mon, 28 Apr 2014 22:13:04 -0700

On Mon, Apr 28, 2014 at 6:52 AM, Pádraig Brady <P <at> draigbrady.com> wrote:
...
>> I see it here too (as the only failure in make check with -fsanitize=address
>
> The attached should address this.

Thanks for taking the time to address that.
The patch looks fine, modulo what looks like an accidentally-added
empty line all by itself in one chunk, just before this comment:

  /* Define the refence field, if any.  */

Previous Next

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