From unknown Wed Aug 20 06:40:21 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#17455 <17455@debbugs.gnu.org> To: bug#17455 <17455@debbugs.gnu.org> Subject: Status: [PATCH] shred: fix overflow checking of command-line options Reply-To: bug#17455 <17455@debbugs.gnu.org> Date: Wed, 20 Aug 2025 13:40:21 +0000 retitle 17455 [PATCH] shred: fix overflow checking of command-line options reassign 17455 coreutils submitter 17455 Paul Eggert severity 17455 normal tag 17455 patch thanks From debbugs-submit-bounces@debbugs.gnu.org Sat May 10 14:50:08 2014 Received: (at submit) by debbugs.gnu.org; 10 May 2014 18:50:08 +0000 Received: from localhost ([127.0.0.1]:58317 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjCLW-0001HN-QO for submit@debbugs.gnu.org; Sat, 10 May 2014 14:50:07 -0400 Received: from eggs.gnu.org ([208.118.235.92]:39510) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjCLU-0001Gj-87 for submit@debbugs.gnu.org; Sat, 10 May 2014 14:50:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WjCLG-0006XQ-5x for submit@debbugs.gnu.org; Sat, 10 May 2014 14:49:59 -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]:54799) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjCLG-0006XM-37 for submit@debbugs.gnu.org; Sat, 10 May 2014 14:49:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41213) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjCL9-0002IV-UE for bug-coreutils@gnu.org; Sat, 10 May 2014 14:49:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WjCL3-0006Wd-QL for bug-coreutils@gnu.org; Sat, 10 May 2014 14:49:43 -0400 Received: from kiwi.cs.ucla.edu ([131.179.128.19]:50020) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WjCL3-0006WV-IB for bug-coreutils@gnu.org; Sat, 10 May 2014 14:49:37 -0400 Received: from kiwi.cs.ucla.edu (localhost.cs.ucla.edu [127.0.0.1]) by kiwi.cs.ucla.edu (8.14.5+Sun/8.14.5/UCLACS-6.0) with ESMTP id s4AInZea026727 for ; Sat, 10 May 2014 11:49:35 -0700 (PDT) Received: (from eggert@localhost) by kiwi.cs.ucla.edu (8.14.5+Sun/8.14.5/Submit) id s4AInZJj026726 for bug-coreutils@gnu.org; Sat, 10 May 2014 11:49:35 -0700 (PDT) Message-Id: <201405101849.s4AInZJj026726@kiwi.cs.ucla.edu> From: Paul Eggert Date: Sat, 10 May 2014 11:42:38 -0700 Subject: [PATCH] shred: fix overflow checking of command-line options To: bug-coreutils@gnu.org X-detected-operating-system: by eggs.gnu.org: Solaris 10 X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2001:4830:134:3::11 X-Spam-Score: -4.0 (----) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 (----) * src/shred.c (main): Limit -n (number of passes) value to ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long. Limit the -s (file size) value to OFF_T_MAX. --- src/shred.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/shred.c b/src/shred.c index 607c6be..f4347e0 100644 --- a/src/shred.c +++ b/src/shred.c @@ -1231,7 +1231,7 @@ main (int argc, char **argv) { uintmax_t tmp; if (xstrtoumax (optarg, NULL, 10, &tmp, NULL) != LONGINT_OK - || MIN (UINT32_MAX, SIZE_MAX / sizeof (int)) < tmp) + || MIN (ULONG_MAX, SIZE_MAX / sizeof (int)) <= tmp) { error (EXIT_FAILURE, 0, _("%s: invalid number of passes"), quotearg_colon (optarg)); @@ -1256,9 +1256,10 @@ main (int argc, char **argv) case 's': { - uintmax_t tmp; - if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") - != LONGINT_OK) + intmax_t tmp; + if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") + != LONGINT_OK) + || OFF_T_MAX < tmp) { error (EXIT_FAILURE, 0, _("%s: invalid file size"), quotearg_colon (optarg)); -- 1.9.0 From debbugs-submit-bounces@debbugs.gnu.org Sat May 10 14:52:00 2014 Received: (at control) by debbugs.gnu.org; 10 May 2014 18:52:00 +0000 Received: from localhost ([127.0.0.1]:58322 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjCNM-0001L5-79 for submit@debbugs.gnu.org; Sat, 10 May 2014 14:52:00 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]:58306) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjCNJ-0001Km-8P for control@debbugs.gnu.org; Sat, 10 May 2014 14:51:58 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 66DDAA6000C for ; Sat, 10 May 2014 11:51:51 -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 7O8lJdLWN95X for ; Sat, 10 May 2014 11:51:43 -0700 (PDT) Received: from [192.168.1.9] (pool-108-0-233-62.lsanca.fios.verizon.net [108.0.233.62]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id E593439E8012 for ; Sat, 10 May 2014 11:51:42 -0700 (PDT) Message-ID: <536E753E.9010005@cs.ucla.edu> Date: Sat, 10 May 2014 11:51:42 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: control@debbugs.gnu.org Subject: this bug is fixed Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.0 (---) X-Debbugs-Envelope-To: control X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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: -3.0 (---) close 17455 From debbugs-submit-bounces@debbugs.gnu.org Sat May 10 15:40:14 2014 Received: (at 17455) by debbugs.gnu.org; 10 May 2014 19:40:14 +0000 Received: from localhost ([127.0.0.1]:58365 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjD81-00056O-Cz for submit@debbugs.gnu.org; Sat, 10 May 2014 15:40:13 -0400 Received: from mail-yk0-f174.google.com ([209.85.160.174]:40851) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjD7y-000565-7l for 17455@debbugs.gnu.org; Sat, 10 May 2014 15:40:10 -0400 Received: by mail-yk0-f174.google.com with SMTP id 9so4656571ykp.33 for <17455@debbugs.gnu.org>; Sat, 10 May 2014 12:40:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc:content-type; bh=cWK2I1ngrs91AsylQsVT9IGRqwU0yTSg17YULyXUars=; b=C1PDJhAaqGVOl5wssk6CQsQu3eLeG5ZUDzXoInktZlfRhLN3P8ALk7md8qFV4l/Of7 lEC35nZD4YMBGDp2OEYaD13jbY6rNqcXk9Cx803vdJx5+bb0SAeuSZqXSVTa7NfD9F8i UAy4QogmC5V6zfe5gz2wGBu6FYrHO7NBuIIUlCeADNXQ+twtusA4SLxbLiPqe5zRSdLb jQBQGRlaEgT+P04zxlQY07YR2kWEbozTtnPTplXZjERKlvBjx8QykrptHt2ZG1zyXhNK CGzix6PhG4sca8OMTufHNeui8uO7RrZIdLKshjlrNPD10PKYqBOJuZHNqRqVxQmZ16yT czkQ== X-Received: by 10.236.43.227 with SMTP id l63mr26862206yhb.144.1399750804646; Sat, 10 May 2014 12:40:04 -0700 (PDT) MIME-Version: 1.0 Received: by 10.170.127.18 with HTTP; Sat, 10 May 2014 12:39:44 -0700 (PDT) In-Reply-To: <201405101849.s4AInZJj026726@kiwi.cs.ucla.edu> References: <201405101849.s4AInZJj026726@kiwi.cs.ucla.edu> From: Jim Meyering Date: Sat, 10 May 2014 12:39:44 -0700 X-Google-Sender-Auth: lMVWQQdP_39mgJZoKmlrTOJXLus Message-ID: Subject: Re: bug#17455: [PATCH] shred: fix overflow checking of command-line options To: Paul Eggert Content-Type: multipart/mixed; boundary=089e0160a7567c478704f910e1e9 X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 17455 Cc: 17455@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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 (/) --089e0160a7567c478704f910e1e9 Content-Type: text/plain; charset=ISO-8859-1 On Sat, May 10, 2014 at 11:42 AM, Paul Eggert wrote: > * src/shred.c (main): Limit -n (number of passes) value to > ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long. > Limit the -s (file size) value to OFF_T_MAX. > --- > src/shred.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/src/shred.c b/src/shred.c > index 607c6be..f4347e0 100644 > --- a/src/shred.c > +++ b/src/shred.c ... > @@ -1256,9 +1256,10 @@ main (int argc, char **argv) > > case 's': > { > - uintmax_t tmp; > - if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > - != LONGINT_OK) > + intmax_t tmp; > + if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > + != LONGINT_OK) > + || OFF_T_MAX < tmp) Hi Paul, The above makes it so shred now accepts a negative size. Before, that would be diagnosed as invalid: $ shred -s-1 k shred: -1: invalid file size With a size of -2, shred will write 64KB blocks forever -- or until it runs out of space. Here's a patch to fix that and to add a test covering that case: --089e0160a7567c478704f910e1e9 Content-Type: application/octet-stream; name="0001-shred-don-t-infloop-upon-negative-size.patch" Content-Disposition: attachment; filename="0001-shred-don-t-infloop-upon-negative-size.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_hv1bgeuz0 RnJvbSBkN2NmY2JlZjdlYjJjZDEyYWM4M2U1YzFjMTIzZGUyMDE1YWRiZWY5IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBKaW0gTWV5ZXJpbmcgPG1leWVyaW5nQGZiLmNvbT4KRGF0ZTog U2F0LCAxMCBNYXkgMjAxNCAxMjozNjoxNiAtMDcwMApTdWJqZWN0OiBbUEFUQ0hdIHNocmVkOiBk b24ndCBpbmZsb29wIHVwb24gbmVnYXRpdmUgc2l6ZQoKKiBzcmMvc2hyZWQuYyAobWFpbik6IFdp dGggdGhlIHByZWNlZGluZyBjaGFuZ2UsIHNocmVkIC1zLTIgRklMRQp3b3VsZCB3cml0ZSA2NEtC IGJsb2NrcyBmb3JldmVyIC0tIG9yIHVudGlsIGRpc2sgZnVsbC4gVGhpcyBjaGFuZ2UKbWFrZXMg c2hyZWQgcmVqZWN0IGEgbmVnYXRpdmUgc2l6ZS4KKiB0ZXN0cy9taXNjL3NocmVkLW5lZ2F0aXZl LnNoOiBOZXcgZmlsZS4KKiB0ZXN0cy9sb2NhbC5tayAoYWxsX3Rlc3RzKTogQWRkIGl0LgotLS0K IHNyYy9zaHJlZC5jICAgICAgICAgICAgICAgICAgfCAgNCArKy0tCiB0ZXN0cy9sb2NhbC5tayAg ICAgICAgICAgICAgIHwgIDEgKwogdGVzdHMvbWlzYy9zaHJlZC1uZWdhdGl2ZS5zaCB8IDI4ICsr KysrKysrKysrKysrKysrKysrKysrKysrKysKIDMgZmlsZXMgY2hhbmdlZCwgMzEgaW5zZXJ0aW9u cygrKSwgMiBkZWxldGlvbnMoLSkKIGNyZWF0ZSBtb2RlIDEwMDc1NSB0ZXN0cy9taXNjL3NocmVk LW5lZ2F0aXZlLnNoCgpkaWZmIC0tZ2l0IGEvc3JjL3NocmVkLmMgYi9zcmMvc2hyZWQuYwppbmRl eCBmNDM0N2UwLi5iZDg4ZTM4IDEwMDY0NAotLS0gYS9zcmMvc2hyZWQuYworKysgYi9zcmMvc2hy ZWQuYwpAQCAtMTI1Niw4ICsxMjU2LDggQEAgbWFpbiAoaW50IGFyZ2MsIGNoYXIgKiphcmd2KQoK ICAgICAgICAgY2FzZSAncyc6CiAgICAgICAgICAgewotICAgICAgICAgICAgaW50bWF4X3QgdG1w OwotICAgICAgICAgICAgaWYgKCh4c3RydG9pbWF4IChvcHRhcmcsIE5VTEwsIDAsICZ0bXAsICJj YkJrS01HVFBFWlkwIikKKyAgICAgICAgICAgIHVpbnRtYXhfdCB0bXA7CisgICAgICAgICAgICBp ZiAoKHhzdHJ0b3VtYXggKG9wdGFyZywgTlVMTCwgMCwgJnRtcCwgImNiQmtLTUdUUEVaWTAiKQog ICAgICAgICAgICAgICAgICAhPSBMT05HSU5UX09LKQogICAgICAgICAgICAgICAgIHx8IE9GRl9U X01BWCA8IHRtcCkKICAgICAgICAgICAgICAgewpkaWZmIC0tZ2l0IGEvdGVzdHMvbG9jYWwubWsg Yi90ZXN0cy9sb2NhbC5tawppbmRleCA1Mjg2YmZiLi5jZDdkYTViIDEwMDY0NAotLS0gYS90ZXN0 cy9sb2NhbC5taworKysgYi90ZXN0cy9sb2NhbC5tawpAQCAtMzEzLDYgKzMxMyw3IEBAIGFsbF90 ZXN0cyA9CQkJCQlcCiAgIHRlc3RzL21pc2Mvc2hhMzg0c3VtLnBsCQkJXAogICB0ZXN0cy9taXNj L3NoYTUxMnN1bS5wbAkJCVwKICAgdGVzdHMvbWlzYy9zaHJlZC1leGFjdC5zaAkJCVwKKyAgdGVz dHMvbWlzYy9zaHJlZC1uZWdhdGl2ZS5zaAkJCVwKICAgdGVzdHMvbWlzYy9zaHJlZC1wYXNzZXMu c2gJCQlcCiAgIHRlc3RzL21pc2Mvc2hyZWQtcmVtb3ZlLnNoCQkJXAogICB0ZXN0cy9taXNjL3No dWYuc2gJCQkJXApkaWZmIC0tZ2l0IGEvdGVzdHMvbWlzYy9zaHJlZC1uZWdhdGl2ZS5zaCBiL3Rl c3RzL21pc2Mvc2hyZWQtbmVnYXRpdmUuc2gKbmV3IGZpbGUgbW9kZSAxMDA3NTUKaW5kZXggMDAw MDAwMC4uODZjYmYzZQotLS0gL2Rldi9udWxsCisrKyBiL3Rlc3RzL21pc2Mvc2hyZWQtbmVnYXRp dmUuc2gKQEAgLTAsMCArMSwyOCBAQAorIyEvYmluL3NoCisjIEV4ZXJjaXNlIHNocmVkIC1zLTMg RklMRQorCisjIENvcHlyaWdodCAoQykgMjAxNCBGcmVlIFNvZnR3YXJlIEZvdW5kYXRpb24sIElu Yy4KKworIyBUaGlzIHByb2dyYW0gaXMgZnJlZSBzb2Z0d2FyZTogeW91IGNhbiByZWRpc3RyaWJ1 dGUgaXQgYW5kL29yIG1vZGlmeQorIyBpdCB1bmRlciB0aGUgdGVybXMgb2YgdGhlIEdOVSBHZW5l cmFsIFB1YmxpYyBMaWNlbnNlIGFzIHB1Ymxpc2hlZCBieQorIyB0aGUgRnJlZSBTb2Z0d2FyZSBG b3VuZGF0aW9uLCBlaXRoZXIgdmVyc2lvbiAzIG9mIHRoZSBMaWNlbnNlLCBvcgorIyAoYXQgeW91 ciBvcHRpb24pIGFueSBsYXRlciB2ZXJzaW9uLgorCisjIFRoaXMgcHJvZ3JhbSBpcyBkaXN0cmli dXRlZCBpbiB0aGUgaG9wZSB0aGF0IGl0IHdpbGwgYmUgdXNlZnVsLAorIyBidXQgV0lUSE9VVCBB TlkgV0FSUkFOVFk7IHdpdGhvdXQgZXZlbiB0aGUgaW1wbGllZCB3YXJyYW50eSBvZgorIyBNRVJD SEFOVEFCSUxJVFkgb3IgRklUTkVTUyBGT1IgQSBQQVJUSUNVTEFSIFBVUlBPU0UuICBTZWUgdGhl CisjIEdOVSBHZW5lcmFsIFB1YmxpYyBMaWNlbnNlIGZvciBtb3JlIGRldGFpbHMuCisKKyMgWW91 IHNob3VsZCBoYXZlIHJlY2VpdmVkIGEgY29weSBvZiB0aGUgR05VIEdlbmVyYWwgUHVibGljIExp Y2Vuc2UKKyMgYWxvbmcgd2l0aCB0aGlzIHByb2dyYW0uICBJZiBub3QsIHNlZSA8aHR0cDovL3d3 dy5nbnUub3JnL2xpY2Vuc2VzLz4uCisKKy4gIiR7c3JjZGlyPS59L3Rlc3RzL2luaXQuc2giOyBw YXRoX3ByZXBlbmRfIC4vc3JjCitwcmludF92ZXJfIHNocmVkCisKK2VjaG8gJ3NocmVkOiAtMjog aW52YWxpZCBmaWxlIHNpemUnID4gZXhwIHx8IGZyYW1ld29ya19mYWlsdXJlXworZWNobyAxMjM0 ID4gZiB8fCBmcmFtZXdvcmtfZmFpbHVyZV8KKworc2hyZWQgLXMtMiBmIDI+ZXJyICYmIGZhaWw9 MQorY29tcGFyZSBleHAgZXJyIHx8IGZhaWw9MQorCitFeGl0ICRmYWlsCi0tIAoyLjAuMC5yYzAu MzguZzE2OTdiZjMKCg== --089e0160a7567c478704f910e1e9-- From debbugs-submit-bounces@debbugs.gnu.org Sat May 10 15:54:03 2014 Received: (at 17455) by debbugs.gnu.org; 10 May 2014 19:54:03 +0000 Received: from localhost ([127.0.0.1]:58382 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjDLO-0006eC-3m for submit@debbugs.gnu.org; Sat, 10 May 2014 15:54:02 -0400 Received: from mail5.vodafone.ie ([213.233.128.176]:23704) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjDLL-0006dh-GI for 17455@debbugs.gnu.org; Sat, 10 May 2014 15:54:00 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApQBAI2CblNtT7Bo/2dsb2JhbAANTA7GZYMRAYEtgxkBAQEEMgFGEAsNCwkWDwkDAgECAUUGDQEHAQGIQqsNpSMXjXtXB4RAAQOgVo5vQYFt Received: from unknown (HELO [192.168.1.79]) ([109.79.176.104]) by mail3.vodafone.ie with ESMTP; 10 May 2014 20:53:52 +0100 Message-ID: <536E83D0.6000707@draigBrady.com> Date: Sat, 10 May 2014 20:53:52 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Paul Eggert Subject: Re: bug#17455: [PATCH] shred: fix overflow checking of command-line options References: <201405101849.s4AInZJj026726@kiwi.cs.ucla.edu> In-Reply-To: <201405101849.s4AInZJj026726@kiwi.cs.ucla.edu> X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 17455 Cc: 17455@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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.0 (/) On 05/10/2014 07:42 PM, Paul Eggert wrote: > * src/shred.c (main): Limit -n (number of passes) value to > ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long. > Limit the -s (file size) value to OFF_T_MAX. > --- > src/shred.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/src/shred.c b/src/shred.c > index 607c6be..f4347e0 100644 > --- a/src/shred.c > +++ b/src/shred.c > @@ -1231,7 +1231,7 @@ main (int argc, char **argv) > { > uintmax_t tmp; > if (xstrtoumax (optarg, NULL, 10, &tmp, NULL) != LONGINT_OK > - || MIN (UINT32_MAX, SIZE_MAX / sizeof (int)) < tmp) > + || MIN (ULONG_MAX, SIZE_MAX / sizeof (int)) <= tmp) Cool. I think the UNIT32 came from the first version where ((word32)passes != passes) was used as a hacky way to detect overflow. > { > error (EXIT_FAILURE, 0, _("%s: invalid number of passes"), > quotearg_colon (optarg)); > @@ -1256,9 +1256,10 @@ main (int argc, char **argv) > > case 's': > { > - uintmax_t tmp; > - if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > - != LONGINT_OK) > + intmax_t tmp; > + if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > + != LONGINT_OK) > + || OFF_T_MAX < tmp) > { > error (EXIT_FAILURE, 0, _("%s: invalid file size"), > quotearg_colon (optarg)); > Ever since --size was added, it was stored and interpreted internally as off_t, so the OFF_T_MAX guard is correct. However the xstrtoumax() was used as a guard against negative numbers which are now accepted :( I'll fixup with a revert to that bit. thanks! Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Sat May 10 15:57:18 2014 Received: (at 17455-done) by debbugs.gnu.org; 10 May 2014 19:57:18 +0000 Received: from localhost ([127.0.0.1]:58386 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjDOX-0006k9-7d for submit@debbugs.gnu.org; Sat, 10 May 2014 15:57:17 -0400 Received: from mail5.vodafone.ie ([213.233.128.176]:45756) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjDOV-0006jt-4J for 17455-done@debbugs.gnu.org; Sat, 10 May 2014 15:57:15 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: ApQBAPWDblNtT7Bo/2dsb2JhbAANTINVxi8BgS2DGQEBAQQyAUYQCw0EAwECAQkWDwkDAgECAT0IBg0BBQIBAQWIPasPpGMXjXtXB4RABI8li1+FUoYUiRyBbQ Received: from unknown (HELO [192.168.1.79]) ([109.79.176.104]) by mail3.vodafone.ie with ESMTP; 10 May 2014 20:57:08 +0100 Message-ID: <536E8494.9010602@draigBrady.com> Date: Sat, 10 May 2014 20:57:08 +0100 From: =?ISO-8859-1?Q?P=E1draig_Brady?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#17455: [PATCH] shred: fix overflow checking of command-line options References: <201405101849.s4AInZJj026726@kiwi.cs.ucla.edu> In-Reply-To: X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 17455-done Cc: Paul Eggert , 17455-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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.0 (/) On 05/10/2014 08:39 PM, Jim Meyering wrote: > On Sat, May 10, 2014 at 11:42 AM, Paul Eggert wrote: >> > * src/shred.c (main): Limit -n (number of passes) value to >> > ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long. >> > Limit the -s (file size) value to OFF_T_MAX. >> > --- >> > src/shred.c | 9 +++++---- >> > 1 file changed, 5 insertions(+), 4 deletions(-) >> > >> > diff --git a/src/shred.c b/src/shred.c >> > index 607c6be..f4347e0 100644 >> > --- a/src/shred.c >> > +++ b/src/shred.c > ... >> > @@ -1256,9 +1256,10 @@ main (int argc, char **argv) >> > >> > case 's': >> > { >> > - uintmax_t tmp; >> > - if (xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") >> > - != LONGINT_OK) >> > + intmax_t tmp; >> > + if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") >> > + != LONGINT_OK) >> > + || OFF_T_MAX < tmp) > Hi Paul, > The above makes it so shred now accepts a negative size. > Before, that would be diagnosed as invalid: > > $ shred -s-1 k > shred: -1: invalid file size > > With a size of -2, shred will write 64KB blocks forever -- or until it > runs out of space. > > Here's a patch to fix that and to add a test covering that case: > > > 0001-shred-don-t-infloop-upon-negative-size.patch > > > From d7cfcbef7eb2cd12ac83e5c1c123de2015adbef9 Mon Sep 17 00:00:00 2001 > From: Jim Meyering > Date: Sat, 10 May 2014 12:36:16 -0700 > Subject: [PATCH] shred: don't infloop upon negative size > > * src/shred.c (main): With the preceding change, shred -s-2 FILE > would write 64KB blocks forever -- or until disk full. This change > makes shred reject a negative size. > * tests/misc/shred-negative.sh: New file. > * tests/local.mk (all_tests): Add it. > --- > src/shred.c | 4 ++-- > tests/local.mk | 1 + > tests/misc/shred-negative.sh | 28 ++++++++++++++++++++++++++++ > 3 files changed, 31 insertions(+), 2 deletions(-) > create mode 100755 tests/misc/shred-negative.sh > > diff --git a/src/shred.c b/src/shred.c > index f4347e0..bd88e38 100644 > --- a/src/shred.c > +++ b/src/shred.c > @@ -1256,8 +1256,8 @@ main (int argc, char **argv) > > case 's': > { > - intmax_t tmp; > - if ((xstrtoimax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > + uintmax_t tmp; > + if ((xstrtoumax (optarg, NULL, 0, &tmp, "cbBkKMGTPEZY0") > != LONGINT_OK) > || OFF_T_MAX < tmp) > { > diff --git a/tests/local.mk b/tests/local.mk > index 5286bfb..cd7da5b 100644 > --- a/tests/local.mk > +++ b/tests/local.mk > @@ -313,6 +313,7 @@ all_tests = \ > tests/misc/sha384sum.pl \ > tests/misc/sha512sum.pl \ > tests/misc/shred-exact.sh \ > + tests/misc/shred-negative.sh \ > tests/misc/shred-passes.sh \ > tests/misc/shred-remove.sh \ > tests/misc/shuf.sh \ > diff --git a/tests/misc/shred-negative.sh b/tests/misc/shred-negative.sh > new file mode 100755 > index 0000000..86cbf3e > --- /dev/null > +++ b/tests/misc/shred-negative.sh > @@ -0,0 +1,28 @@ > +#!/bin/sh > +# Exercise shred -s-3 FILE > + > +# Copyright (C) 2014 Free Software Foundation, Inc. > + > +# This program is free software: you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation, either version 3 of the License, or > +# (at your option) any later version. > + > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > + > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src > +print_ver_ shred > + > +echo 'shred: -2: invalid file size' > exp || framework_failure_ > +echo 1234 > f || framework_failure_ > + > +shred -s-2 f 2>err && fail=1 > +compare exp err || fail=1 > + > +Exit $fail > -- 2.0.0.rc0.38.g1697bf3 Ah great you beat me to it :) Code looks good, as does the test. Please push. I've marked this bug as done. thanks! Pádraig. From debbugs-submit-bounces@debbugs.gnu.org Sat May 10 16:06:03 2014 Received: (at 17455) by debbugs.gnu.org; 10 May 2014 20:06:03 +0000 Received: from localhost ([127.0.0.1]:58390 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjDX0-00070s-Se for submit@debbugs.gnu.org; Sat, 10 May 2014 16:06:03 -0400 Received: from smtp.cs.ucla.edu ([131.179.128.62]:60228) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1WjDWy-00070H-LI for 17455@debbugs.gnu.org; Sat, 10 May 2014 16:06:01 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.cs.ucla.edu (Postfix) with ESMTP id 9B02A39E801E; Sat, 10 May 2014 13:05:54 -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 Lzp1oIzzwBc6; Sat, 10 May 2014 13:05:46 -0700 (PDT) Received: from [192.168.1.9] (pool-108-0-233-62.lsanca.fios.verizon.net [108.0.233.62]) by smtp.cs.ucla.edu (Postfix) with ESMTPSA id 143B639E8011; Sat, 10 May 2014 13:05:46 -0700 (PDT) Message-ID: <536E8699.5010704@cs.ucla.edu> Date: Sat, 10 May 2014 13:05:45 -0700 From: Paul Eggert Organization: UCLA Computer Science Department User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-Version: 1.0 To: Jim Meyering Subject: Re: bug#17455: [PATCH] shred: fix overflow checking of command-line options References: <201405101849.s4AInZJj026726@kiwi.cs.ucla.edu> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -3.0 (---) X-Debbugs-Envelope-To: 17455 Cc: 17455@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 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: -3.0 (---) Thanks for catching that! I used signed because I was worried about offbeat hosts where UINTMAX_MAX < OFF_T_MAX, which I think POSIX allows, but I suppose I was going overboard. There are still a few oddball hosts out there (I just the other day discovered that Unisys still ships POSIXish mainframes where UINTMAX_MAX == OFF_T_MAX, because off_t has 40 bits and uintmax_t has 39!) but nothing *that* weird as far as I know. From unknown Wed Aug 20 06:40:21 2025 Received: (at fakecontrol) by fakecontrolmessage; To: internal_control@debbugs.gnu.org From: Debbugs Internal Request Subject: Internal Control Message-Id: bug archived. Date: Sun, 08 Jun 2014 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