Message-ID: <CA+55aFyjp7beQDGKVuFtMLDRJTdE7ypeRAXzxoA9pLWVmjik1Q@mail.gmail.com>
Date:	Thu, 10 Sep 2015 11:37:53 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Alexey Dobriyan <adobriyan@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [patch 27/95] scanf: fix type range overflow

On Thu, Sep 10, 2015 at 11:01 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> Nobody in the history of the world has really cared about integer
> parsing overflow. Really. It's just not an issue. It's not a security
> issue (the possibly truncated number that is returned *could* have
> been returned by just parsing that numebr in the first place), and
> it's not a practical worry either.

Btw, while the type-based range checking cannot possibly be a security
issue (because by definition, you could get the same value some other
way by just picking a valid number instead of relying on overflow),
there could be real security advantages to an interface that made the
actual accepted range be something explicitly given.

If course, the caller can always do *that* check trivially themselves,
but making it part of the interface might force people to think about
their expected valid ranges.

Also, as was shown by the "parse_integer()" failure, the error cases
really must not be mixed up with the returned parsed value, or with
the number of characters used.

The problem, of course, is that the more arguments you add to the
parsing function, the less convenient it gets.

But I could imagine something like

     int parse_integer(string, base, T *, <error-statement>);

could still be usable as an interface. Note that the return value
would be guaranteed to be positive (not zero, not negative), because
the error condition would execute the passed-in statement, so you
could write code like

    str += parse_integer(str, 0, &variable, return -EINVAL);

and it would basically mean

 - parse the integer in "str" with any base
 - if any error happens (no actual integer found), do "return -EINVAL"
 - otherwise, put the result in "variable" and return the length
(which is guaranteed positive)
 - it would not check the range in any shape or form.

and then we could have a helper function somthing like

   #define parse_integer_range(str,base,T,min,max,err) ({
        int __len; \
        typeof *T __tmp; \
        __len = parse_integer(str,base,&__tmp,0,0,err); \
        if (__tmp < (min) || __tmp > (max)) err; else *(T) = __tmp; \
        __len; \
    })

and notice how in the range-checking version, the value still gets
truncated to the type (which fundamentally cannot be a security
issue), but we then - within that type - check for the given actual
range.

So with that kind of setup you can write

     str += parse_integer_range(str, 10, &variable, 0, 5, return -EINVAL);

and know that "variable" will be set to a value between 0 and 5, or
we'll be returning an error.

I dunno. But basically the above seems to me to be *much* more useful
semantics for some actual range checking than any crazy type-based one
that by definition cannot help security, since it doesn't actually
limit the real range that might be assigned.

We might have special rules for "what happens if the error statement
is a fall-through statement". Maybe we can make the rule be that the
return value from "parse_integer()" in that case is zero, so that you
can do things like

     int i = 0, array[MAX] = { 0, };

    while (i < MAX) {
        str += parse_integer(str, 0, array+i, /*nothing*/);
        if (*str != ',')
            break;
        str++;
    }
    if (*str)
        return -EINVAL;

to fill in an array of integers where a missing integer keeps the
previous value (in this case 0). So the above could parse things like
"5,,,6".

The above seems kind of a special case, but I made it up more as an
example of how robust the semantics can be when you *don't* mix up the
value (or the number of chatacters eaten) with the error case.

              Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists