GNU bug report logs - #28120
[PATCH] ptx: fix a possible crash caused by integer overflow

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Kamil Dudka <kdudka <at> redhat.com>
Subject: bug#28120: closed (Re: bug#28120: [PATCH] ptx: fix a possible
 crash caused by integer overflow)
Date: Thu, 17 Aug 2017 19:15:02 +0000

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

Your bug report

#28120: [PATCH] ptx: fix a possible crash caused by integer overflow

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 28120 <at> debbugs.gnu.org.

-- 
28120: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=28120
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Kamil Dudka <kdudka <at> redhat.com>, 28120-done <at> debbugs.gnu.org
Subject: Re: bug#28120: [PATCH] ptx: fix a possible crash caused by integer
 overflow
Date: Thu, 17 Aug 2017 12:14:05 -0700

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

On 08/17/2017 04:40 AM, Kamil Dudka wrote:
> -typedef short int DELTA;	/* to hold displacement within one context */
> +typedef int DELTA;		/* to hold displacement within one context */

Thanks for the heads-up. Although that fixes things for that particular 
test case, it won't work for larger cases. The type should be ptrdiff_t 
instead of int.

As its FIXME comment says, ptx is riddled with integer-overflow bugs. I 
installed the attached patch to fix the bug that you mentioned along 
with the other low-hanging fruit that I found, and am marking the bug as 
fixed upstream. I expect some other integer-overflow bugs can still 
occur in practice, but at least this patch is a significant improvement.

This patch prefers signed integer types like ptrdiff_t to unsigned types 
like size_t, as signed types allow for better checking when compiled 
with sanitization.

[0001-ptx-fix-some-integer-overflow-bugs.patch (text/x-patch, attachment)]

[Message part 5 (message/rfc822, inline)]

From: Kamil Dudka <kdudka <at> redhat.com>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] ptx: fix a possible crash caused by integer overflow
Date: Thu, 17 Aug 2017 13:40:24 +0200

The crash might not be reproducible in all environments but I was able
to observe invalid reads by valgrind while running ptx on the reproducer
for <https://bugzilla.redhat.com/1482445>:

% curl -s 'https://bugzilla.redhat.com/attachment.cgi?id=1314625' | gzip -cd | valgrind ptx > /dev/null
==4288== Memcheck, a memory error detector
==4288== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==4288== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==4288== Command: ptx
==4288==
==4288== Invalid read of size 1
==4288==    at 0x402FA9: define_all_fields (ptx.c:1434)
==4288==    by 0x402FA9: generate_all_output (ptx.c:1780)
==4288==    by 0x402FA9: main (ptx.c:2155)
==4288==  Address 0x5328a6f is 2,287 bytes inside a block of size 16,352 free'd
==4288==    at 0x4C2FC47: realloc (vg_replace_malloc.c:785)
==4288==    by 0x408B65: xrealloc (xmalloc.c:61)
==4288==    by 0x402857: find_occurs_in_text (ptx.c:955)
==4288==    by 0x402857: main (ptx.c:2141)
==4288==  Block was alloc'd at
==4288==    at 0x4C2FC47: realloc (vg_replace_malloc.c:785)
==4288==    by 0x408B65: xrealloc (xmalloc.c:61)
==4288==    by 0x402857: find_occurs_in_text (ptx.c:955)
==4288==    by 0x402857: main (ptx.c:2141)
==4288==
==4288== Invalid read of size 1
==4288==    at 0x4031AB: define_all_fields (ptx.c:1501)
==4288==    by 0x4031AB: generate_all_output (ptx.c:1780)
==4288==    by 0x4031AB: main (ptx.c:2155)
==4288==  Address 0x5328a6f is 2,287 bytes inside a block of size 16,352 free'd
==4288==    at 0x4C2FC47: realloc (vg_replace_malloc.c:785)
==4288==    by 0x408B65: xrealloc (xmalloc.c:61)
==4288==    by 0x402857: find_occurs_in_text (ptx.c:955)
==4288==    by 0x402857: main (ptx.c:2141)
==4288==  Block was alloc'd at
==4288==    at 0x4C2FC47: realloc (vg_replace_malloc.c:785)
==4288==    by 0x408B65: xrealloc (xmalloc.c:61)
==4288==    by 0x402857: find_occurs_in_text (ptx.c:955)
==4288==    by 0x402857: main (ptx.c:2141)
==4288==
==4288==
==4288== HEAP SUMMARY:
==4288==     in use at exit: 2,340,851 bytes in 96 blocks
==4288==   total heap usage: 200 allocs, 104 frees, 6,478,142 bytes allocated
==4288==
==4288== LEAK SUMMARY:
==4288==    definitely lost: 0 bytes in 0 blocks
==4288==    indirectly lost: 0 bytes in 0 blocks
==4288==      possibly lost: 0 bytes in 0 blocks
==4288==    still reachable: 2,340,851 bytes in 96 blocks
==4288==         suppressed: 0 bytes in 0 blocks
==4288== Rerun with --leak-check=full to see details of leaked memory
==4288==
==4288== For counts of detected and suppressed errors, rerun with: -v
==4288== ERROR SUMMARY: 915 errors from 2 contexts (suppressed: 0 from 0)
---
 src/ptx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ptx.c b/src/ptx.c
index c0c9733..f4ed7d4 100644
--- a/src/ptx.c
+++ b/src/ptx.c
@@ -224,7 +224,7 @@ static BLOCK *text_buffers;	/* files to study */
    start of the reference field, it is of type (DELTA) and usually
    negative.  */
 
-typedef short int DELTA;	/* to hold displacement within one context */
+typedef int DELTA;		/* to hold displacement within one context */
 
 typedef struct
   {
-- 
2.9.5

Previous Next

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