[debbugs-tracker] bug#28120: closed ([PATCH] ptx: fix a possible crash caused by integer overflow)

[GNU bug Tracking System]

Your message dated Thu, 17 Aug 2017 12:14:05 -0700
with message-id <address@hidden>
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
address@hidden)


-- 
28120: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=28120
GNU Bug Tracking System
Contact address@hidden with problems
> --- Begin Message ---
>
>
>
> 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
>
>
>
>
> --- End Message ---

--- Begin Message --- Subject: Re: bug#28120: [PATCH] ptx: fix a possible crash caused by integer overflow Date: Thu, 17 Aug 2017 12:14:05 -0700 User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

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
Description: Text Data

--- End Message ---