From unknown Sun Jun 22 17:13:31 2025 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-Mailer: MIME-tools 5.509 (Entity 5.509) Content-Type: text/plain; charset=utf-8 From: bug#28120 <28120@debbugs.gnu.org> To: bug#28120 <28120@debbugs.gnu.org> Subject: Status: [PATCH] ptx: fix a possible crash caused by integer overflow Reply-To: bug#28120 <28120@debbugs.gnu.org> Date: Mon, 23 Jun 2025 00:13:31 +0000 retitle 28120 [PATCH] ptx: fix a possible crash caused by integer overflow reassign 28120 coreutils submitter 28120 Kamil Dudka severity 28120 normal tag 28120 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Thu Aug 17 07:46:53 2017 Received: (at submit) by debbugs.gnu.org; 17 Aug 2017 11:46:53 +0000 Received: from localhost ([127.0.0.1]:42039 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1diJGG-0003dt-Rh for submit@debbugs.gnu.org; Thu, 17 Aug 2017 07:46:53 -0400 Received: from eggs.gnu.org ([208.118.235.92]:60380) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1diJGE-0003dc-Do for submit@debbugs.gnu.org; Thu, 17 Aug 2017 07:46:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1diJG8-0003SK-3a for submit@debbugs.gnu.org; Thu, 17 Aug 2017 07:46:45 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:37557) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1diJG8-0003SC-05 for submit@debbugs.gnu.org; Thu, 17 Aug 2017 07:46:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51324) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1diJG5-0004wx-3t for bug-coreutils@gnu.org; Thu, 17 Aug 2017 07:46:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1diJG0-0003PQ-4D for bug-coreutils@gnu.org; Thu, 17 Aug 2017 07:46:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43487) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1diJFz-0003Oa-R8 for bug-coreutils@gnu.org; Thu, 17 Aug 2017 07:46:36 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B0090CA36B for ; Thu, 17 Aug 2017 11:40:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B0090CA36B Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=kdudka@redhat.com Received: from localhost.localdomain (unknown [10.43.2.37]) by smtp.corp.redhat.com (Postfix) with ESMTP id 09E796016F; Thu, 17 Aug 2017 11:40:24 +0000 (UTC) From: Kamil Dudka To: bug-coreutils@gnu.org Subject: [PATCH] ptx: fix a possible crash caused by integer overflow Date: Thu, 17 Aug 2017 13:40:24 +0200 Message-Id: <20170817114024.27282-1-kdudka@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Thu, 17 Aug 2017 11:40:25 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.1 (----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.1 (----) 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 : % 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 From debbugs-submit-bounces@debbugs.gnu.org Thu Aug 17 15:14:21 2017 Received: (at 28120-done) by debbugs.gnu.org; 17 Aug 2017 19:14:21 +0000 Received: from localhost ([127.0.0.1]:43133 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1diQFI-0006OX-Gs for submit@debbugs.gnu.org; Thu, 17 Aug 2017 15:14:21 -0400 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:50824) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1diQFF-0006OJ-FN for 28120-done@debbugs.gnu.org; Thu, 17 Aug 2017 15:14:18 -0400 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 5EF66160886; Thu, 17 Aug 2017 12:14:11 -0700 (PDT) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id 0tNZ6M1fKtXm; Thu, 17 Aug 2017 12:14:09 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 3A23C16089D; Thu, 17 Aug 2017 12:14:09 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id nQY68BHnYKcC; Thu, 17 Aug 2017 12:14:09 -0700 (PDT) Received: from Penguin.CS.UCLA.EDU (Penguin.CS.UCLA.EDU [131.179.64.200]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 14B02160896; Thu, 17 Aug 2017 12:14:09 -0700 (PDT) Subject: Re: bug#28120: [PATCH] ptx: fix a possible crash caused by integer overflow To: Kamil Dudka , 28120-done@debbugs.gnu.org References: <20170817114024.27282-1-kdudka@redhat.com> From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: <27dd791a-1672-5f6e-f56f-f2af448c5a87@cs.ucla.edu> 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 MIME-Version: 1.0 In-Reply-To: <20170817114024.27282-1-kdudka@redhat.com> Content-Type: multipart/mixed; boundary="------------7D5E9F26328218D6F5B673C9" Content-Language: en-US X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 28120-done X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.7 (/) This is a multi-part message in MIME format. --------------7D5E9F26328218D6F5B673C9 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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. --------------7D5E9F26328218D6F5B673C9 Content-Type: text/x-patch; name="0001-ptx-fix-some-integer-overflow-bugs.patch" Content-Disposition: attachment; filename="0001-ptx-fix-some-integer-overflow-bugs.patch" Content-Transfer-Encoding: quoted-printable >From 1d9765a764790cc68ec52c16d7ccbf633c404f0e Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 17 Aug 2017 12:02:16 -0700 Subject: [PATCH] ptx: fix some integer overflow bugs MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit Problem reported by Lukas Zachar at: http://bugzilla.redhat.com/1482445 * src/ptx.c (line_width, gap_size, maximum_word_length) (reference_max_width, half_line_width, before_max_width) (keyafter_max_width, truncation_string_length, compare_words) (compare_occurs, search_table, find_occurs_in_text, print_spaces) (fix_output_parameters, define_all_fields): Use ptrdiff_t, not int, for object offsets and sizes. (WORD, OCCURS): Use ptrdiff_t, not short int. (WORD_TABLE, number_of_occurs, generate_all_output): Prefer ptrdiff_t to size_t where either will do. (total_line_count, file_line_count, OCCURS, fix_output_parameters) (define_all_fields): Use intmax_t, not int, for line counts. (DELTA): Remove. All uses changed. (OCCURS, find_occurs_in_text, fix_output_parameters): Use int, not size_t, for file indexes. (tail_truncation, before_truncation, keyafter_truncation) (head_truncation, search_table, define_all_fields) (generate_all_output): Use bool for booleans. (digest_word_file, find_occurs_in_text): Use x2nrealloc instead of checking for overflow by hand. (find_occurs_in_text, fix_output_parameters, define_all_fields): Omit unnecessary cast. (fix_output_parameters): Don=E2=80=99t assume integers fit in 11 digits. (fix_output_parameters, define_all_fields): Use sprintf return value rather than calling strlen. (define_all_fields): Do not rely on sprintf to generate a string that may contain more than INT_MAX bytes. (main): Use xstrtoimax, not xstrtoul. Use xnmalloc to catch integer overflow. --- src/ptx.c | 194 +++++++++++++++++++++++++++++---------------------------= ------ 1 file changed, 91 insertions(+), 103 deletions(-) diff --git a/src/ptx.c b/src/ptx.c index c0c9733a2..2aababff6 100644 --- a/src/ptx.c +++ b/src/ptx.c @@ -59,9 +59,9 @@ /* Global definitions. */ =20 /* FIXME: There are many unchecked integer overflows in this file, - that will cause this command to misbehave given large inputs or - options. Many of the "int" values below should be "size_t" or - something else like that. */ + and in theory they could cause this command to have undefined + behavior given large inputs or options. This command should + diagnose any such overflow and exit. */ =20 /* Program options. */ =20 @@ -77,8 +77,8 @@ static bool gnu_extensions =3D true; /* trigger all GNU= extensions */ static bool auto_reference =3D false; /* refs are 'file_name:line_number= :' */ static bool input_reference =3D false; /* refs at beginning of input lin= es */ static bool right_reference =3D false; /* output refs after right contex= t */ -static int line_width =3D 72; /* output line width in characters */ -static int gap_size =3D 3; /* number of spaces between output fields */ +static ptrdiff_t line_width =3D 72; /* output line width in characters *= / +static ptrdiff_t gap_size =3D 3; /* number of spaces between output fiel= ds */ static const char *truncation_string =3D "/"; /* string used to mark line truncations = */ static const char *macro_name =3D "xx"; /* macro name for roff or TeX ou= tput */ @@ -105,8 +105,8 @@ static struct regex_data context_regex; /* end of con= text */ static struct regex_data word_regex; /* keyword */ =20 /* A BLOCK delimit a region in memory of arbitrary size, like the copy o= f a - whole file. A WORD is something smaller, its length should fit in a - short integer. A WORD_TABLE may contain several WORDs. */ + whole file. A WORD is similar, except it is intended for smaller reg= ions. + A WORD_TABLE may contain several WORDs. */ =20 typedef struct { @@ -118,7 +118,7 @@ BLOCK; typedef struct { char *start; /* pointer to beginning of region */ - short int size; /* length of the region */ + ptrdiff_t size; /* length of the region */ } WORD; =20 @@ -126,7 +126,7 @@ typedef struct { WORD *start; /* array of WORDs */ size_t alloc; /* allocated length */ - size_t length; /* number of used entries */ + ptrdiff_t length; /* number of used entries */ } WORD_TABLE; =20 @@ -149,10 +149,10 @@ static struct re_registers word_regs; static char word_fastmap[CHAR_SET_SIZE]; =20 /* Maximum length of any word read. */ -static int maximum_word_length; +static ptrdiff_t maximum_word_length; =20 /* Maximum width of any reference used. */ -static int reference_max_width; +static ptrdiff_t reference_max_width; =20 /* Ignore and Only word tables. */ =20 @@ -162,9 +162,9 @@ static WORD_TABLE only_table; /* table of words to s= elect */ /* Source text table, and scanning macros. */ =20 static int number_input_files; /* number of text input files */ -static int total_line_count; /* total number of lines seen so far */ +static intmax_t total_line_count; /* total number of lines seen so far *= / static const char **input_file_name; /* array of text input file names *= / -static int *file_line_count; /* array of 'total_line_count' values at en= d */ +static intmax_t *file_line_count; /* array of line count values at end *= / =20 static BLOCK *text_buffers; /* files to study */ =20 @@ -219,20 +219,18 @@ static BLOCK *text_buffers; /* files to study */ =20 When automatic references are used, the 'reference' value is the overall line number in all input files read so far, in this case, it - is of type (int). When input references are used, the 'reference' + is of type intmax_t. When input references are used, the 'reference' value indicates the distance between the keyword beginning and the - start of the reference field, it is of type (DELTA) and usually + start of the reference field, and it fits in ptrdiff_t and is usually negative. */ =20 -typedef short int DELTA; /* to hold displacement within one context */ - typedef struct { WORD key; /* description of the keyword */ - DELTA left; /* distance to left context start */ - DELTA right; /* distance to right context end */ - int reference; /* reference descriptor */ - size_t file_index; /* corresponding file */ + ptrdiff_t left; /* distance to left context start */ + ptrdiff_t right; /* distance to right context end */ + intmax_t reference; /* reference descriptor */ + int file_index; /* corresponding file */ } OCCURS; =20 @@ -241,7 +239,7 @@ OCCURS; =20 static OCCURS *occurs_table[1]; /* all words retained from the read text= */ static size_t occurs_alloc[1]; /* allocated size of occurs_table */ -static size_t number_of_occurs[1]; /* number of used slots in occurs_tab= le */ +static ptrdiff_t number_of_occurs[1]; /* number of used slots in occurs_= table */ =20 =20 /* Communication among output routines. */ @@ -249,10 +247,17 @@ static size_t number_of_occurs[1]; /* number of use= d slots in occurs_table */ /* Indicate if special output processing is requested for each character= . */ static char edited_flag[CHAR_SET_SIZE]; =20 -static int half_line_width; /* half of line width, reference excluded */ -static int before_max_width; /* maximum width of before field */ -static int keyafter_max_width; /* maximum width of keyword-and-after fie= ld */ -static int truncation_string_length;/* length of string that flags trunc= ation */ +/* Half of line width, reference excluded. */ +static ptrdiff_t half_line_width; + +/* Maximum width of before field. */ +static ptrdiff_t before_max_width; + +/* Maximum width of keyword-and-after field. */ +static ptrdiff_t keyafter_max_width; + +/* Length of string that flags truncation. */ +static ptrdiff_t truncation_string_length; =20 /* When context is limited by lines, wraparound may happen on final outp= ut: the 'head' pointer gives access to some supplementary left context wh= ich @@ -261,16 +266,16 @@ static int truncation_string_length;/* length of st= ring that flags truncation */ beginning of the output line. */ =20 static BLOCK tail; /* tail field */ -static int tail_truncation; /* flag truncation after the tail field */ +static bool tail_truncation; /* flag truncation after the tail field */ =20 static BLOCK before; /* before field */ -static int before_truncation; /* flag truncation before the before field= */ +static bool before_truncation; /* flag truncation before the before fiel= d */ =20 static BLOCK keyafter; /* keyword-and-after field */ -static int keyafter_truncation; /* flag truncation after the keyafter fi= eld */ +static bool keyafter_truncation; /* flag truncation after the keyafter f= ield */ =20 static BLOCK head; /* head field */ -static int head_truncation; /* flag truncation before the head field */ +static bool head_truncation; /* flag truncation before the head field */ =20 static BLOCK reference; /* reference field for input reference mode */ =20 @@ -540,8 +545,8 @@ compare_words (const void *void_first, const void *vo= id_second) { #define first ((const WORD *) void_first) #define second ((const WORD *) void_second) - int length; /* minimum of two lengths */ - int counter; /* cursor in words */ + ptrdiff_t length; /* minimum of two lengths */ + ptrdiff_t counter; /* cursor in words */ int value; /* value of comparison */ =20 length =3D first->size < second->size ? first->size : second->size; @@ -567,7 +572,7 @@ compare_words (const void *void_first, const void *vo= id_second) } } =20 - return first->size - second->size; + return first->size < second->size ? -1 : first->size > second->size; #undef first #undef second } @@ -586,21 +591,21 @@ compare_occurs (const void *void_first, const void = *void_second) int value; =20 value =3D compare_words (&first->key, &second->key); - return value =3D=3D 0 ? first->key.start - second->key.start : value; + return (value ? value + : first->key.start < second->key.start ? -1 + : first->key.start > second->key.start); #undef first #undef second } =20 -/*------------------------------------------------------------. -| Return !0 if WORD appears in TABLE. Uses a binary search. | -`------------------------------------------------------------*/ +/* True if WORD appears in TABLE. Uses a binary search. */ =20 -static int _GL_ATTRIBUTE_PURE +static bool _GL_ATTRIBUTE_PURE search_table (WORD *word, WORD_TABLE *table) { - int lowest; /* current lowest possible index */ - int highest; /* current highest possible index */ - int middle; /* current middle index */ + ptrdiff_t lowest; /* current lowest possible index */ + ptrdiff_t highest; /* current highest possible index */ + ptrdiff_t middle; /* current middle index */ int value; /* value from last comparison */ =20 lowest =3D 0; @@ -614,9 +619,9 @@ search_table (WORD *word, WORD_TABLE *table) else if (value > 0) lowest =3D middle + 1; else - return 1; + return true; } - return 0; + return false; } =20 /*---------------------------------------------------------------------. @@ -713,14 +718,8 @@ digest_word_file (const char *file_name, WORD_TABLE = *table) if (cursor > word_start) { if (table->length =3D=3D table->alloc) - { - if ((SIZE_MAX / sizeof *table->start - 1) / 2 < table->all= oc) - xalloc_die (); - table->alloc =3D table->alloc * 2 + 1; - table->start =3D xrealloc (table->start, - table->alloc * sizeof *table->sta= rt); - } - + table->start =3D x2nrealloc (table->start, &table->alloc, + sizeof *table->start); table->start[table->length].start =3D word_start; table->start[table->length].size =3D cursor - word_start; table->length++; @@ -744,13 +743,13 @@ digest_word_file (const char *file_name, WORD_TABLE= *table) `----------------------------------------------------------------------*= / =20 static void -find_occurs_in_text (size_t file_index) +find_occurs_in_text (int file_index) { char *cursor; /* for scanning the source text */ char *scan; /* for scanning the source text also */ char *line_start; /* start of the current input line */ char *line_scan; /* newlines scanned until this point */ - int reference_length; /* length of reference in input mode */ + ptrdiff_t reference_length; /* length of reference in input mode */ WORD possible_key; /* possible key, to ease searches */ OCCURS *occurs_cursor; /* current OCCURS under construction */ =20 @@ -946,16 +945,9 @@ find_occurs_in_text (size_t file_index) where it will be constructed. */ =20 if (number_of_occurs[0] =3D=3D occurs_alloc[0]) - { - if ((SIZE_MAX / sizeof *occurs_table[0] - 1) / 2 - < occurs_alloc[0]) - xalloc_die (); - occurs_alloc[0] =3D occurs_alloc[0] * 2 + 1; - occurs_table[0] =3D - xrealloc (occurs_table[0], - occurs_alloc[0] * sizeof *occurs_table[0]); - } - + occurs_table[0] =3D x2nrealloc (occurs_table[0], + &occurs_alloc[0], + sizeof *occurs_table[0]); occurs_cursor =3D occurs_table[0] + number_of_occurs[0]; =20 /* Define the reference field, if any. */ @@ -990,8 +982,7 @@ find_occurs_in_text (size_t file_index) of the reference. The reference position is simply the value of 'line_start'. */ =20 - occurs_cursor->reference - =3D (DELTA) (line_start - possible_key.start); + occurs_cursor->reference =3D line_start - possible_key.sta= rt; if (reference_length > reference_max_width) reference_max_width =3D reference_length; } @@ -1023,11 +1014,9 @@ find_occurs_in_text (size_t file_index) `-----------------------------------------*/ =20 static void -print_spaces (int number) +print_spaces (ptrdiff_t number) { - int counter; - - for (counter =3D number; counter > 0; counter--) + for (ptrdiff_t counter =3D number; counter > 0; counter--) putchar (' '); } =20 @@ -1200,10 +1189,9 @@ print_field (BLOCK field) static void fix_output_parameters (void) { - size_t file_index; /* index in text input file arrays */ - int line_ordinal; /* line ordinal value for reference */ - char ordinal_string[12]; /* edited line ordinal for reference */ - int reference_width; /* width for the whole reference */ + int file_index; /* index in text input file arrays */ + intmax_t line_ordinal; /* line ordinal value for reference */ + ptrdiff_t reference_width; /* width for the whole reference */ int character; /* character ordinal */ const char *cursor; /* cursor in some constant strings */ =20 @@ -1219,15 +1207,15 @@ fix_output_parameters (void) line_ordinal =3D file_line_count[file_index] + 1; if (file_index > 0) line_ordinal -=3D file_line_count[file_index - 1]; - sprintf (ordinal_string, "%d", line_ordinal); - reference_width =3D strlen (ordinal_string); + char ordinal_string[INT_BUFSIZE_BOUND (intmax_t)]; + reference_width =3D sprintf (ordinal_string, "%"PRIdMAX, line_= ordinal); if (input_file_name[file_index]) reference_width +=3D strlen (input_file_name[file_index]); if (reference_width > reference_max_width) reference_max_width =3D reference_width; } reference_max_width++; - reference.start =3D xmalloc ((size_t) reference_max_width + 1); + reference.start =3D xmalloc (reference_max_width + 1); } =20 /* If the reference appears to the left of the output line, reserve so= me @@ -1355,14 +1343,14 @@ fix_output_parameters (void) static void define_all_fields (OCCURS *occurs) { - int tail_max_width; /* allowable width of tail field */ - int head_max_width; /* allowable width of head field */ + ptrdiff_t tail_max_width; /* allowable width of tail field */ + ptrdiff_t head_max_width; /* allowable width of head field */ char *cursor; /* running cursor in source text */ char *left_context_start; /* start of left context */ char *right_context_end; /* end of right context */ char *left_field_start; /* conservative start for 'head'/'before' */ const char *file_name; /* file name for reference */ - int line_ordinal; /* line ordinal for reference */ + intmax_t line_ordinal; /* line ordinal for reference */ const char *buffer_start; /* start of buffered file for this occurs */ const char *buffer_end; /* end of buffered file for this occurs */ =20 @@ -1435,7 +1423,7 @@ define_all_fields (OCCURS *occurs) before_truncation =3D cursor > left_context_start; } else - before_truncation =3D 0; + before_truncation =3D false; =20 SKIP_WHITE (before.start, buffer_end); =20 @@ -1468,11 +1456,11 @@ define_all_fields (OCCURS *occurs) =20 if (tail.end > tail.start) { - keyafter_truncation =3D 0; + keyafter_truncation =3D false; tail_truncation =3D truncation_string && tail.end < right_cont= ext_end; } else - tail_truncation =3D 0; + tail_truncation =3D false; =20 SKIP_WHITE_BACKWARDS (tail.end, tail.start); } @@ -1483,7 +1471,7 @@ define_all_fields (OCCURS *occurs) =20 tail.start =3D NULL; tail.end =3D NULL; - tail_truncation =3D 0; + tail_truncation =3D false; } =20 /* 'head' could not take more columns than what has been left in the r= ight @@ -1506,12 +1494,12 @@ define_all_fields (OCCURS *occurs) =20 if (head.end > head.start) { - before_truncation =3D 0; + before_truncation =3D false; head_truncation =3D (truncation_string && head.start > left_context_start); } else - head_truncation =3D 0; + head_truncation =3D false; =20 SKIP_WHITE (head.start, head.end); } @@ -1522,7 +1510,7 @@ define_all_fields (OCCURS *occurs) =20 head.start =3D NULL; head.end =3D NULL; - head_truncation =3D 0; + head_truncation =3D false; } =20 if (auto_reference) @@ -1540,8 +1528,8 @@ define_all_fields (OCCURS *occurs) if (occurs->file_index > 0) line_ordinal -=3D file_line_count[occurs->file_index - 1]; =20 - sprintf (reference.start, "%s:%d", file_name, line_ordinal); - reference.end =3D reference.start + strlen (reference.start); + char *file_end =3D stpcpy (reference.start, file_name); + reference.end =3D file_end + sprintf (file_end, ":%"PRIdMAX, line_= ordinal); } else if (input_reference) { @@ -1549,7 +1537,7 @@ define_all_fields (OCCURS *occurs) /* Reference starts at saved position for reference and extends ri= ght until some white space is met. */ =20 - reference.start =3D keyafter.start + (DELTA) occurs->reference; + reference.start =3D keyafter.start + occurs->reference; reference.end =3D reference.start; SKIP_NON_WHITE (reference.end, right_context_end); } @@ -1753,7 +1741,7 @@ output_one_dumb_line (void) static void generate_all_output (void) { - size_t occurs_index; /* index of keyword entry being processed */ + ptrdiff_t occurs_index; /* index of keyword entry being processed */ OCCURS *occurs_cursor; /* current keyword entry being processed */ =20 /* The following assignments are useful to provide default values in c= ase @@ -1762,11 +1750,11 @@ generate_all_output (void) =20 tail.start =3D NULL; tail.end =3D NULL; - tail_truncation =3D 0; + tail_truncation =3D false; =20 head.start =3D NULL; head.end =3D NULL; - head_truncation =3D 0; + head_truncation =3D false; =20 /* Loop over all keyword occurrences. */ =20 @@ -1946,12 +1934,12 @@ main (int argc, char **argv) =20 case 'g': { - unsigned long int tmp_ulong; - if (xstrtoul (optarg, NULL, 0, &tmp_ulong, NULL) !=3D LONGIN= T_OK - || ! (0 < tmp_ulong && tmp_ulong <=3D INT_MAX)) + intmax_t tmp; + if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) =3D=3D LONGI= NT_OK + && 0 < tmp && tmp <=3D PTRDIFF_MAX)) die (EXIT_FAILURE, 0, _("invalid gap width: %s"), quote (optarg)); - gap_size =3D tmp_ulong; + gap_size =3D tmp; break; } =20 @@ -1973,12 +1961,12 @@ main (int argc, char **argv) =20 case 'w': { - unsigned long int tmp_ulong; - if (xstrtoul (optarg, NULL, 0, &tmp_ulong, NULL) !=3D LONGIN= T_OK - || ! (0 < tmp_ulong && tmp_ulong <=3D INT_MAX)) + intmax_t tmp; + if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) =3D=3D LONGI= NT_OK + && 0 < tmp && tmp <=3D PTRDIFF_MAX)) die (EXIT_FAILURE, 0, _("invalid line width: %s"), quote (optarg)); - line_width =3D tmp_ulong; + line_width =3D tmp; break; } =20 @@ -2045,9 +2033,9 @@ main (int argc, char **argv) else if (gnu_extensions) { number_input_files =3D argc - optind; - input_file_name =3D xmalloc (number_input_files * sizeof *input_fi= le_name); - file_line_count =3D xmalloc (number_input_files * sizeof *file_lin= e_count); - text_buffers =3D xmalloc (number_input_files * sizeof *text_buf= fers); + input_file_name =3D xnmalloc (number_input_files, sizeof *input_fi= le_name); + file_line_count =3D xnmalloc (number_input_files, sizeof *file_lin= e_count); + text_buffers =3D xnmalloc (number_input_files, sizeof *text_buf= fers); =20 for (file_index =3D 0; file_index < number_input_files; file_index= ++) { --=20 2.13.5 --------------7D5E9F26328218D6F5B673C9-- From debbugs-submit-bounces@debbugs.gnu.org Fri Aug 18 04:17:16 2017 Received: (at submit) by debbugs.gnu.org; 18 Aug 2017 08:17:16 +0000 Received: from localhost ([127.0.0.1]:43480 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dicSx-0002ww-Nu for submit@debbugs.gnu.org; Fri, 18 Aug 2017 04:17:15 -0400 Received: from eggs.gnu.org ([208.118.235.92]:49693) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dicSw-0002wh-AY for submit@debbugs.gnu.org; Fri, 18 Aug 2017 04:17:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dicSq-0000id-15 for submit@debbugs.gnu.org; Fri, 18 Aug 2017 04:17:08 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:38629) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dicSp-0000iZ-Tz for submit@debbugs.gnu.org; Fri, 18 Aug 2017 04:17:07 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40650) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dicSn-0005oY-4Q for bug-coreutils@gnu.org; Fri, 18 Aug 2017 04:17:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dicSi-0000gU-6u for bug-coreutils@gnu.org; Fri, 18 Aug 2017 04:17:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46063) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dicSi-0000g6-0S for bug-coreutils@gnu.org; Fri, 18 Aug 2017 04:17:00 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D487BC047B61; Fri, 18 Aug 2017 08:16:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D487BC047B61 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=kdudka@redhat.com Received: from kdudka-nb.localnet (unknown [10.43.2.37]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6D81C4D72A; Fri, 18 Aug 2017 08:16:58 +0000 (UTC) From: Kamil Dudka To: Paul Eggert Subject: Re: bug#28120: [PATCH] ptx: fix a possible crash caused by integer overflow Date: Fri, 18 Aug 2017 10:17:08 +0200 Message-ID: <1663834.DVYOClfZ8W@kdudka-nb> User-Agent: KMail/4.14.10 (Linux/4.12.5-gentoo; KDE/4.14.32; x86_64; ; ) In-Reply-To: <27dd791a-1672-5f6e-f56f-f2af448c5a87@cs.ucla.edu> References: <20170817114024.27282-1-kdudka@redhat.com> <27dd791a-1672-5f6e-f56f-f2af448c5a87@cs.ucla.edu> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 18 Aug 2017 08:16:59 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.1 (----) X-Debbugs-Envelope-To: submit Cc: bug-coreutils@gnu.org, 28120-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.1 (----) On Thursday, August 17, 2017 12:14:05 Paul Eggert wrote: > 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. Do you have an example of the larger case? We could add a test-case for it. > 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. Your patch introduces the following warnings: Error: CONSTANT_EXPRESSION_RESULT: src/ptx.c:1939: result_independent_of_operands: "tmp <= 9223372036854775807L" is always true regardless of the values of its operands. This occurs as the logical second operand of "&&". # 1937| intmax_t tmp; # 1938| if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) == LONGINT_OK # 1939|-> && 0 < tmp && tmp <= PTRDIFF_MAX)) # 1940| die (EXIT_FAILURE, 0, _("invalid gap width: %s"), # 1941| quote (optarg)); Error: CONSTANT_EXPRESSION_RESULT: src/ptx.c:1966: result_independent_of_operands: "tmp <= 9223372036854775807L" is always true regardless of the values of its operands. This occurs as the logical second operand of "&&". # 1964| intmax_t tmp; # 1965| if (! (xstrtoimax (optarg, NULL, 0, &tmp, NULL) == LONGINT_OK # 1966|-> && 0 < tmp && tmp <= PTRDIFF_MAX)) # 1967| die (EXIT_FAILURE, 0, _("invalid line width: %s"), # 1968| quote (optarg)); Anyway, it fixes the original bug so I am fine with the patch as it is. Thank you for pushing the fix! Kamil From debbugs-submit-bounces@debbugs.gnu.org Fri Aug 18 11:44:27 2017 Received: (at submit) by debbugs.gnu.org; 18 Aug 2017 15:44:27 +0000 Received: from localhost ([127.0.0.1]:44539 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dijRi-00076C-RG for submit@debbugs.gnu.org; Fri, 18 Aug 2017 11:44:27 -0400 Received: from eggs.gnu.org ([208.118.235.92]:58760) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dijRh-00075v-7L for submit@debbugs.gnu.org; Fri, 18 Aug 2017 11:44:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dijRb-0005MX-AU for submit@debbugs.gnu.org; Fri, 18 Aug 2017 11:44:20 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=disabled version=3.3.2 Received: from lists.gnu.org ([2001:4830:134:3::11]:54055) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dijRb-0005MN-6l for submit@debbugs.gnu.org; Fri, 18 Aug 2017 11:44:19 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49707) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dijRZ-0000md-UY for bug-coreutils@gnu.org; Fri, 18 Aug 2017 11:44:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dijRW-0005KO-QC for bug-coreutils@gnu.org; Fri, 18 Aug 2017 11:44:17 -0400 Received: from zimbra.cs.ucla.edu ([131.179.128.68]:45460) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dijRW-0005JZ-Kk for bug-coreutils@gnu.org; Fri, 18 Aug 2017 11:44:14 -0400 Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 11FE9160895; Fri, 18 Aug 2017 08:44:12 -0700 (PDT) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id JcLn4FKfHZI5; Fri, 18 Aug 2017 08:44:11 -0700 (PDT) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 67373160897; Fri, 18 Aug 2017 08:44:11 -0700 (PDT) X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id TiMyF11ymV83; Fri, 18 Aug 2017 08:44:11 -0700 (PDT) Received: from [192.168.1.9] (unknown [47.153.184.153]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 43D72160888; Fri, 18 Aug 2017 08:44:11 -0700 (PDT) Subject: Re: bug#28120: [PATCH] ptx: fix a possible crash caused by integer overflow To: Kamil Dudka References: <20170817114024.27282-1-kdudka@redhat.com> <27dd791a-1672-5f6e-f56f-f2af448c5a87@cs.ucla.edu> <1663834.DVYOClfZ8W@kdudka-nb> From: Paul Eggert Organization: UCLA Computer Science Department Message-ID: <572e1f41-886c-904c-133e-91d5cb730c9d@cs.ucla.edu> Date: Fri, 18 Aug 2017 08:44:11 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1663834.DVYOClfZ8W@kdudka-nb> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.0 (----) X-Debbugs-Envelope-To: submit Cc: bug-coreutils@gnu.org, 28120-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -4.0 (----) Kamil Dudka wrote: > Do you have an example of the larger case? We could add a test-case for it. I don't have an example. It would have to be pretty large, e.g., input containing more than 2**31 lines. > Your patch introduces the following warnings: These are Coverity warnings, right? They are false alarms, as usual for Coverity with this sort of thing. From unknown Sun Jun 22 17:13:31 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Sat, 16 Sep 2017 11:24:04 +0000 User-Agent: Fakemail v42.6.9 # This is a fake control message. # # The action: # bug archived. thanks # This fakemail brought to you by your local debbugs # administrator