Messages in this thread | | | Date | Thu, 10 Sep 2015 11:37:53 -0700 | Subject | Re: [patch 27/95] scanf: fix type range overflow | From | Linus Torvalds <> |
| |
On Thu, Sep 10, 2015 at 11:01 AM, Linus Torvalds <torvalds@linux-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
| |