On Wed, May 13, 2015 at 02:45:56PM +0100, John Keeping wrote: > On Wed, May 13, 2015 at 02:41:24PM +0100, John Keeping wrote: > > On Wed, May 13, 2015 at 03:35:29PM +0200, Jason A. Donenfeld wrote: > > > Anybody have any objections to this? In some cases it's slightly more > > > verbose, but otherwise, I can't see any downsides. > > > > It's worse if there is trailing data. Since there's nothing obvious we > > can do if the input is bad, I'm not sure how much we care (i.e. ignoring > > the return value from strtol_i is OK) but whereas atoi will parse a > > valid value followed by trailing garbage strtol_i will just fail. > > > > Worse than that, if it fails it leaves the result uninitialized, which > > doesn't matter in the cases where we just update a variable, but at > > least one part of this patch introduces a new variable that is not set > > if strtol_i fails. > > Oops... I didn't double-check the patch before sending, it does always > initialize the variables first so the only worry is trailing garbage. > > Of course, if atoi leads to SQL injection, what makes strtol safe? The > test seems fundamentally useless; AFAICT the whole point is that if I > parse some user input and use without validating it then I can end up > doing something like: > > int num_items = <user input>; > items = malloc(num_items * sizeof(*items)); > > leading to integer overflow. But the mechanism used to convert the user > input from a string to an integer is completely irrelevant.
Exactly. I'm not familiar with the utility, but can nessus have exceptions to a rule? In cgit's case, I don't see the use of atoi() as a problem in this context. _______________________________________________ CGit mailing list CGit@lists.zx2c4.com http://lists.zx2c4.com/mailman/listinfo/cgit