Re: [PATCH nft] scanner: treat invalid octal strings as strings

[Jeremy Sowden <jeremy@azazel.net>]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

On 2022-12-22, at 11:52:49 +0100, Pablo Neira Ayuso wrote:
> On Thu, Dec 22, 2022 at 11:38:39AM +0100, Pablo Neira Ayuso wrote:
> > On Fri, Dec 16, 2022 at 08:27:14PM +0000, Jeremy Sowden wrote:
> [...]
> > > We get:
> > > 
> > >   $ sudo ./src/nft -f - <<<'
> > >   > table x {
> > >   >   chain y {
> > >   >     ip saddr 0308 continue comment "error"
> > >   >   }
> > >   > }
> > >   > '
> > >   /dev/stdin:4:14-17: Error: Could not resolve hostname: Name or service not known
> > >       ip saddr 0308 continue comment "error"
> > >                ^^^^
> > > 
> > > Add a test-case.
> > 
> > Applied, thanks.
> > 
> > I am sorry I missed this patch before the release.
> 
> Hm. I thought this patch just fixes the parsing of octals.
>
> iptables and iproute seem to support for octals?

So does nft.  However, 0308 is not valid octal, and nft was silently
truncating it to 030.

For hex and decimal, we know that the entire number string is valid in
the base and only have to worry whether it is too long and may result in
a out-of-range error.  For octal, there is also the possibility that the
string may contain 8 or 9.  This patch adds a check for this and if the
check fails the failure is handled as an error in the same way it would
be if strtoull had reported `ERANGE`.

I did consider adding an `{octalstring}` match to handle octal
separately from decimal, but in the end the solution in this patch
seemed simpler.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]