bug#17455: [PATCH] shred: fix overflow checking of command-line options

[Pádraig Brady]

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.