[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#17455: [PATCH] shred: fix overflow checking of command-line options
From: |
Pádraig Brady |
Subject: |
bug#17455: [PATCH] shred: fix overflow checking of command-line options |
Date: |
Sat, 10 May 2014 20:57:08 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 |
On 05/10/2014 08:39 PM, Jim Meyering wrote:
> On Sat, May 10, 2014 at 11:42 AM, Paul Eggert <address@hidden> 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 <address@hidden>
> 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 <http://www.gnu.org/licenses/>.
> +
> +. "${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.