[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:53:52 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 |
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.