[PATCHv2] ifconfig: prefix length handling fixes for -A

[Erik Auerswald]

Hi,

On Wed, Apr 24, 2024 at 06:11:23PM +0200, Erik Auerswald wrote:
> On Wed, Apr 24, 2024 at 02:55:41PM +0200, Simon Josefsson wrote:
> > Erik Auerswald <auerswal@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.

I have done this in the attached patch.

> 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.

I have done that in the attached patch.

I plan to push the changes in a couple of days, unless I receive negative
feedback.

Best regards,
Erik

0001-ifconfig-prefix-length-handling-fixes-for-A.patch
Description: Text Data