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: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#28120: closed ([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 message dated Thu, 17 Aug 2017 12:14:05 -0700
with message-id <27dd791a-1672-5f6e-f56f-f2af448c5a87 <at> cs.ucla.edu>
and subject line Re: bug#28120: [PATCH] ptx: fix a possible crash caused by integer overflow
has caused the debbugs.gnu.org bug report #28120,
regarding [PATCH] ptx: fix a possible crash caused by integer overflow
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> 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: 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

[Message part 3 (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 4 (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)]

Previous Next

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