From unknown Mon Aug 11 21:12:23 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#7213 <7213@debbugs.gnu.org> To: bug#7213 <7213@debbugs.gnu.org> Subject: Status: [PATCH] sort: fix buffer overrun on 32-bit hosts when warning re obsolete keys Reply-To: bug#7213 <7213@debbugs.gnu.org> Date: Tue, 12 Aug 2025 04:12:23 +0000 retitle 7213 [PATCH] sort: fix buffer overrun on 32-bit hosts when warning = re obsolete keys reassign 7213 coreutils submitter 7213 Paul Eggert severity 7213 normal tag 7213 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 14 03:09:17 2010 Received: (at submit) by debbugs.gnu.org; 14 Oct 2010 07:09:17 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6HwC-000408-S4 for submit@debbugs.gnu.org; Thu, 14 Oct 2010 03:09:17 -0400 Received: from eggs.gnu.org ([140.186.70.92]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6HwA-000400-7W for submit@debbugs.gnu.org; Thu, 14 Oct 2010 03:09:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P6HzZ-0003T4-E0 for submit@debbugs.gnu.org; Thu, 14 Oct 2010 03:12:46 -0400 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on eggs.gnu.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,T_RP_MATCHES_RCVD autolearn=unavailable version=3.3.1 Received: from lists.gnu.org ([199.232.76.165]:44800) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P6HzY-0003Sw-Dp for submit@debbugs.gnu.org; Thu, 14 Oct 2010 03:12:45 -0400 Received: from [140.186.70.92] (port=38940 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1P6HzR-0006vq-7a for bug-coreutils@gnu.org; Thu, 14 Oct 2010 03:12:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1P6HzF-0003Qq-Ez for bug-coreutils@gnu.org; Thu, 14 Oct 2010 03:12:26 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]:50603) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1P6HzF-0003Qk-9y for bug-coreutils@gnu.org; Thu, 14 Oct 2010 03:12:25 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 5814139E80DF for ; Thu, 14 Oct 2010 00:12:24 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YAoAGU2VTEQ0 for ; Thu, 14 Oct 2010 00:12:23 -0700 (PDT) Received: from [192.168.1.10] (pool-71-189-109-235.lsanca.fios.verizon.net [71.189.109.235]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id D71F439E80DB for ; Thu, 14 Oct 2010 00:12:23 -0700 (PDT) Message-ID: <4CB6AD57.4030207@cs.ucla.edu> Date: Thu, 14 Oct 2010 00:12:23 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.12) Gecko/20100915 Thunderbird/3.0.8 MIME-Version: 1.0 To: bug-coreutils@gnu.org Subject: [PATCH] sort: fix buffer overrun on 32-bit hosts when warning re obsolete keys Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6, seldom 2.4 (older, 4) X-Spam-Score: -4.6 (----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -4.6 (----) * src/sort.c (key_warnings): Local buffer should be of size INT_BUFSIZE_BOUND (uintmax_t), not INT_BUFSIZE_BOUND (sword). This bug was discovered by running 'make check' on a 32-bit Solaris 8 sparc host, using Sun cc. I saw several other instances of invoking umaxtostr on a buffer declared to be of size INT_BUFSIZE_BOUND (VAR), and these instances should at some point be replaced by INT_BUFSIZE_BOUND (uintmax_t) too, as that's a less error-prone style. --- src/sort.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/sort.c b/src/sort.c index c155eda..7e25f6a 100644 --- a/src/sort.c +++ b/src/sort.c @@ -2320,7 +2320,7 @@ key_warnings (struct keyfield const *gkey, bool gkey_only) { size_t sword = key->sword; size_t eword = key->eword; - char tmp[INT_BUFSIZE_BOUND (sword)]; + char tmp[INT_BUFSIZE_BOUND (uintmax_t)]; /* obsolescent syntax +A.x -B.y is equivalent to: -k A+1.x+1,B.y (when y = 0) -k A+1.x+1,B+1.y (when y > 0) */ -- 1.7.2 From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 14 05:33:49 2010 Received: (at 7213-done) by debbugs.gnu.org; 14 Oct 2010 09:33:49 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6KC4-0006QA-ME for submit@debbugs.gnu.org; Thu, 14 Oct 2010 05:33:49 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6KC2-0006Q0-I7 for 7213-done@debbugs.gnu.org; Thu, 14 Oct 2010 05:33:47 -0400 Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id 9988FE362; Thu, 14 Oct 2010 11:37:17 +0200 (CEST) From: Jim Meyering To: Paul Eggert Subject: Re: bug#7213: [PATCH] sort: fix buffer overrun on 32-bit hosts when warning re obsolete keys In-Reply-To: <4CB6AD57.4030207@cs.ucla.edu> (Paul Eggert's message of "Thu, 14 Oct 2010 00:12:23 -0700") References: <4CB6AD57.4030207@cs.ucla.edu> Date: Thu, 14 Oct 2010 11:37:17 +0200 Message-ID: <87tykpqfn6.fsf@meyering.net> Lines: 108 MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: -5.4 (-----) X-Debbugs-Envelope-To: 7213-done Cc: 7213-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -5.4 (-----) Paul Eggert wrote: > * src/sort.c (key_warnings): Local buffer should be of size > INT_BUFSIZE_BOUND (uintmax_t), not INT_BUFSIZE_BOUND (sword). > This bug was discovered by running 'make check' on a 32-bit > Solaris 8 sparc host, using Sun cc. I saw several other instances > of invoking umaxtostr on a buffer declared to be of size > INT_BUFSIZE_BOUND (VAR), and these instances should at some point > be replaced by INT_BUFSIZE_BOUND (uintmax_t) too, as that's a > less error-prone style. > --- > src/sort.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/src/sort.c b/src/sort.c > index c155eda..7e25f6a 100644 > --- a/src/sort.c > +++ b/src/sort.c > @@ -2320,7 +2320,7 @@ key_warnings (struct keyfield const *gkey, bool gkey_only) > { > size_t sword = key->sword; > size_t eword = key->eword; > - char tmp[INT_BUFSIZE_BOUND (sword)]; > + char tmp[INT_BUFSIZE_BOUND (uintmax_t)]; > /* obsolescent syntax +A.x -B.y is equivalent to: > -k A+1.x+1,B.y (when y = 0) > -k A+1.x+1,B+1.y (when y > 0) */ Thanks! Pushed. BTW, for fyi-style patches like this, please use coreutils@gnu.org rather than bug-... Using the latter creates a new bug report in the database and typically requires manual closing, even if only by my appending "-done" to the number part of the Cc'd address above. While I do see how hard-coding the maximum of uintmax_t would make maintenance a little less error-prone, I prefer to use the variable name when possible, since that makes the bound tight, and it is guaranteed to grow if ever the variable's type size increases. However, inttostr.h does not provide a sizetostr (or size_?ttostr) function, so currently we cannot make the bound tight for a variable of type size_t. To that end, I realized that we can automatically detect some classes of abuse like what you fixed above, perhaps even "most" of it, since most users of inttostr.h-declared functions also use a 2nd argument that is declared on the stack, as above. See below: PS. for fyi-style patches like this, please use coreutils@gnu.org rather than bug-coreutils. Using the latter creates a new bug report in the database and typically requires manual closing, even if only by my appending "-done" to the number part of the Cc'd address above. With the following patch, compilation now fails on x86-based systems: sort.c: In function 'key_warnings': sort.c:2335: error: negative width in bit-field 'verify_error_if_negative_size__' sort.c:2335: error: negative width in bit-field 'verify_error_if_negative_size__' sort.c:2336: error: negative width in bit-field 'verify_error_if_negative_size__' sort.c:2336: error: negative width in bit-field 'verify_error_if_negative_size__' sort.c:2339: error: negative width in bit-field 'verify_error_if_negative_size__' sort.c:2339: error: negative width in bit-field 'verify_error_if_negative_size__' sort.c:2340: error: negative width in bit-field 'verify_error_if_negative_size__' sort.c:2340: error: negative width in bit-field 'verify_error_if_negative_size__' I'll add comments. diff --git a/lib/inttostr.h b/lib/inttostr.h index 4f74968..757ac30 100644 --- a/lib/inttostr.h +++ b/lib/inttostr.h @@ -21,6 +21,7 @@ #include #include "intprops.h" +#include "verify.h" #ifndef __GNUC_PREREQ # if defined __GNUC__ && defined __GNUC_MINOR__ @@ -44,3 +45,30 @@ char *inttostr (int, char *) __attribute_warn_unused_result__; char *offtostr (off_t, char *) __attribute_warn_unused_result__; char *uinttostr (unsigned int, char *) __attribute_warn_unused_result__; char *umaxtostr (uintmax_t, char *) __attribute_warn_unused_result__; + +#ifndef inttype +# define imaxtostr(n,s) \ + ((void) verify_true (sizeof (s) == sizeof (char *) \ + || INT_BUFSIZE_BOUND (intmax_t) <= sizeof (s)), \ + (imaxtostr) (n, s)) + +# define inttostr(n,s) \ + ((void) verify_true (sizeof (s) == sizeof (char *) \ + || INT_BUFSIZE_BOUND (int) <= sizeof (s)), \ + (inttostr) (n, s)) + +# define offtostr(n,s) \ + ((void) verify_true (sizeof (s) == sizeof (char *) \ + || INT_BUFSIZE_BOUND (off_t) <= sizeof (s)), \ + (offtostr) (n, s)) + +# define uinttostr(n,s) \ + ((void) verify_true (sizeof (s) == sizeof (char *) \ + || INT_BUFSIZE_BOUND (unsigned int) <= sizeof (s)), \ + (uinttostr) (n, s)) + +# define umaxtostr(n,s) \ + ((void) verify_true (sizeof (s) == sizeof (char *) \ + || INT_BUFSIZE_BOUND (uintmax_t) <= sizeof (s)), \ + (umaxtostr) (n, s)) +#endif From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 14 06:25:57 2010 Received: (at 7213) by debbugs.gnu.org; 14 Oct 2010 10:25:57 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6L0X-0006nm-8x for submit@debbugs.gnu.org; Thu, 14 Oct 2010 06:25:57 -0400 Received: from mail1.slb.deg.dub.stisp.net ([84.203.253.98]) by debbugs.gnu.org with smtp (Exim 4.69) (envelope-from ) id 1P6L0U-0006ng-7l for 7213@debbugs.gnu.org; Thu, 14 Oct 2010 06:25:54 -0400 Received: (qmail 2138 invoked from network); 14 Oct 2010 10:29:22 -0000 Received: from unknown (HELO ?192.168.2.25?) (84.203.137.218) by mail1.slb.deg.dub.stisp.net with SMTP; 14 Oct 2010 10:29:22 -0000 Message-ID: <4CB6DB0C.4010104@draigBrady.com> Date: Thu, 14 Oct 2010 11:27:24 +0100 From: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3 MIME-Version: 1.0 To: Paul Eggert Subject: Re: bug#7213: [PATCH] sort: fix buffer overrun on 32-bit hosts when warning re obsolete keys References: <4CB6AD57.4030207@cs.ucla.edu> In-Reply-To: <4CB6AD57.4030207@cs.ucla.edu> X-Enigmail-Version: 1.0.1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Score: -2.7 (--) X-Debbugs-Envelope-To: 7213 Cc: 7213@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -2.7 (--) Ah I wasn't aware anytostr put the numbers at the end of the buffer. That's confirmed by replacing the tmp buffer with one on the heap and running: $ valgrind ./src/sort --debug +0 -1 /dev/null ==25943== Memcheck, a memory error detector. ==25943== Invalid write of size 1 ==25943== at 0x8051F25: umaxtostr (anytostr.c:34) ==25943== by 0x8050D95: main (sort.c:2336) ==25943== Address 0x4026f64 is 9 bytes after a block of size 11 alloc'd On 14/10/10 08:12, Paul Eggert wrote: > * src/sort.c (key_warnings): Local buffer should be of size > INT_BUFSIZE_BOUND (uintmax_t), not INT_BUFSIZE_BOUND (sword). > This bug was discovered by running 'make check' on a 32-bit > Solaris 8 sparc host, using Sun cc. So the test failed due to buffer overrun side effects? thanks! Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 14 12:33:45 2010 Received: (at 7213) by debbugs.gnu.org; 14 Oct 2010 16:33:46 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6QkT-00033l-Dc for submit@debbugs.gnu.org; Thu, 14 Oct 2010 12:33:45 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6QkQ-00033c-Ky for 7213@debbugs.gnu.org; Thu, 14 Oct 2010 12:33:44 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id DA9B639E80DF; Thu, 14 Oct 2010 09:37:14 -0700 (PDT) X-Virus-Scanned: amavisd-new at smtp.cs.ucla.edu Received: from smtp.cs.ucla.edu ([127.0.0.1]) by localhost (smtp.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tuAe8lLIELUw; Thu, 14 Oct 2010 09:37:13 -0700 (PDT) Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id B64FD39E80DC; Thu, 14 Oct 2010 09:37:13 -0700 (PDT) Message-ID: <4CB731B4.1000000@cs.ucla.edu> Date: Thu, 14 Oct 2010 09:37:08 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.12) Gecko/20100915 Thunderbird/3.0.8 MIME-Version: 1.0 To: =?UTF-8?B?UMOhZHJhaWcgQnJhZHk=?= Subject: Re: bug#7213: [PATCH] sort: fix buffer overrun on 32-bit hosts when warning re obsolete keys References: <4CB6AD57.4030207@cs.ucla.edu> <4CB6DB0C.4010104@draigBrady.com> In-Reply-To: <4CB6DB0C.4010104@draigBrady.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -3.4 (---) X-Debbugs-Envelope-To: 7213 Cc: 7213@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -3.4 (---) On 10/14/10 03:27, P=C3=A1draig Brady wrote: > So the test failed due to buffer overrun side effects? I think so, yes, though I didn't investigate the details. On 10/14/10 02:37, Jim Meyering wrote: > With the following patch, compilation now fails on x86-based systems: >=20 > sort.c: In function 'key_warnings': > sort.c:2335: error: negative width in bit-field 'verify_error_if_negati= ve_size__' > sort.c:2335: error: negative width in bit-field 'verify_error_if_negati= ve_size__' ... I assume this is against the unpatched sort.c. It's nice that it generates a diagnostic, but why is it generating duplicate diagnostics for each error? > BTW, for fyi-style patches like this, > please use coreutils@gnu.org rather than bug-... Sorry about posting to bug-coreutils; I forgot that I was supposed to send it to coreutils. But even if I had remembered, I thought I was supposed to send patches to coreutils only if I had applied them, under the theory that the bug had already been fixed. So the real rule is: send patches to coreutils, and bug reports without patches to bug-coreutils? From debbugs-submit-bounces@debbugs.gnu.org Thu Oct 14 15:45:24 2010 Received: (at 7213) by debbugs.gnu.org; 14 Oct 2010 19:45:24 +0000 Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6Tjv-0004Xq-VR for submit@debbugs.gnu.org; Thu, 14 Oct 2010 15:45:24 -0400 Received: from mx.meyering.net ([82.230.74.64]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1P6Tjt-0004Xi-Sb for 7213@debbugs.gnu.org; Thu, 14 Oct 2010 15:45:22 -0400 Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id 5E9ECE491; Thu, 14 Oct 2010 21:48:54 +0200 (CEST) From: Jim Meyering To: Paul Eggert Subject: Re: bug#7213: [PATCH] sort: fix buffer overrun on 32-bit hosts when warning re obsolete keys In-Reply-To: <4CB731B4.1000000@cs.ucla.edu> (Paul Eggert's message of "Thu, 14 Oct 2010 09:37:08 -0700") References: <4CB6AD57.4030207@cs.ucla.edu> <4CB6DB0C.4010104@draigBrady.com> <4CB731B4.1000000@cs.ucla.edu> Date: Thu, 14 Oct 2010 21:48:54 +0200 Message-ID: <871v7spnbt.fsf@meyering.net> Lines: 41 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -5.4 (-----) X-Debbugs-Envelope-To: 7213 Cc: 7213@debbugs.gnu.org, =?utf-8?Q?P=C3=A1draig?= Brady X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: debbugs-submit-bounces@debbugs.gnu.org Errors-To: debbugs-submit-bounces@debbugs.gnu.org X-Spam-Score: -5.4 (-----) Paul Eggert wrote: > On 10/14/10 03:27, P=C3=A1draig Brady wrote: >> So the test failed due to buffer overrun side effects? > > I think so, yes, though I didn't investigate the details. > > On 10/14/10 02:37, Jim Meyering wrote: >> With the following patch, compilation now fails on x86-based systems: >> >> sort.c: In function 'key_warnings': >> sort.c:2335: error: negative width in bit-field 'verify_error_if_negativ= e_size__' >> sort.c:2335: error: negative width in bit-field 'verify_error_if_negativ= e_size__' > ... > > I assume this is against the unpatched sort.c. It's nice that Right. > it generates a diagnostic, but why is it generating duplicate > diagnostics for each error? That surprised me, too. umaxtostr is used there as an argument to stpcpy, po =3D stpcpy (stpcpy (po, "+"), umaxtostr (sword, tmp)); and stpcpy happens to be a macro on glibc-based systems. >> BTW, for fyi-style patches like this, >> please use coreutils@gnu.org rather than bug-... > > Sorry about posting to bug-coreutils; I forgot that I was > supposed to send it to coreutils. But even if I had remembered, > I thought I was supposed to send patches to coreutils only if I > had applied them, under the theory that the bug had already been > fixed. So the real rule is: send patches to coreutils, and > bug reports without patches to bug-coreutils? Actually, you're welcome to send them to either, but if you send them to bug-coreutils, please close the ticket once your patch has been pushed. From unknown Mon Aug 11 21:12:23 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Fri, 12 Nov 2010 12: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