Hi Simon,
On Wed, Apr 24, 2024 at 02:55:41PM +0200, Simon Josefsson wrote:
> Erik Auerswald <auers...@unix-ag.uni-kl.de> writes:
> >
> > while "ifconfig -A" now accepts CIDR notation, it does not reject
> > prefix length values outside of [0,32]. Also, with a prefix length
> > of 0, undefined begavior is invoked, and at least on x86_64 a wrong
> > netmask is computed.
> >
> > I think the attached patch fixes this.
> >
> > If it is OK, I can commit the changes. What do you think?
>
> Makes sense to me -- when using strtol() one ought to check for LONG_MIN
> and LONG_MAX too, but your comparison address that.
>
> The strtol() function returns the result of the conversion,
> unless the value would underflow or overflow. If an underflow
> occurs, strtol() returns LONG_MIN.
>
> Given that, I would re-order the if statement to compare "n" before
> comparing (stale) "end", although I suppose this is somewhat cosmetic.
This first check tests if strtol() found a leading digit, i.e., if the
value of "n" comes from the textual representation of a number.
If there were no digits at all, strtol() stores the original value
of nptr in *endptr (and returns 0).
The other two checks then verify that the number is in the valid range.
Thus I prefer this order of checks.
This still allows wrong input, e.g., "192.0.2.47/3k". I'll consider
tightening the checks, possibly based on the following from the strtol(3)
man page:
In particular, if *nptr is not '\0' but **endptr is '\0' on return,
the entire string is valid.
This would still allow strange values like "192.0.2.47/ +24", but I do
not think it is worth the effort to detect this.
> Maybe add some other odd values in the self-test? Like
> 1212237832782387238723823782 and -1238912x1298129. Thanks for adding
> regression checks!
I can do that, no problem.
Thanks for the quick feedback! :-)
Br,
Erik
