ok deraadt
Klemens Nanni <[email protected]> wrote:
> On Wed, Mar 27, 2019 at 12:34:52PM +0100, Petr Hoffmann wrote:
> > I noticed it is possible to specify an invalid netmask,
> > e.g. 1.1.1.1/10/20 and still get the address loaded into a table. I
> > conjecture this was introduced by the following change:
> >
> > a7ede25358dad545e0342d2a9f8ef6ce68c6df66
> > Zap bits in host_v4(), use mask parameter
> That was me:
>
> revision 1.326
> date: 2018/07/31 22:48:04; author: kn; state: Exp; lines: +10 -11
> Zap v4mask and v6mask in host()
>
> Simply defer checks whether a mask has been specified to where it's set
> in
> host_*(); this is to reduce address family specific code.
>
> OK sashan
>
> > It looks like the author missed the path addresses are loaded by pfctl's '-T
> > add' command. I guess the '/20' is dropped in host() and then '/10' is
> > processed within host_ip() by inet_net_pton() so no error is reported.
> Good find, thanks.
>
> > OLD:
> > # pfctl -t tableta -T add 1.1.1.1/10/20
> > 1 table created.
> > 1/1 addresses added.
> >
> > NEW:
> > # $PFCTL -t tableta -T add
> > 1.1.1.1/10/20
> > netmask is invalid: /10/20
> Yup; this only affects tables, though. For the ruleset parser, strings
> with more than one "/" already fail the `xhost' production in parse.y:
>
> $ pfctl -vnf-
> pass to 1/2/3
> stdin:1: syntax error
>
> > diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
> > index ee3c2926f1a..5737846123d 100644
> > --- a/sbin/pfctl/pfctl_parser.c
> > +++ b/sbin/pfctl/pfctl_parser.c
> > @@ -1627,7 +1627,7 @@ host(const char *s, int opts)
> > if_name++;
> > }
> >
> > - if ((p = strrchr(ps, '/')) != NULL) {
> > + if ((p = strchr(ps, '/')) != NULL) {
> > mask = strtonum(p+1, 0, 128, &errstr);
> > if (errstr) {
> > fprintf(stderr, "netmask is %s: %s\n", errstr, p);
> OK kn; anyone else?
>
