On Fri, May 17, 2024 at 06:53:34PM +0300, Vitaliy Makkoveev wrote:
> On Fri, May 17, 2024 at 12:44:35PM +0000, Carsten Beckmann wrote:
> > Hi,
> > 
> > pfkeyv2_sysctl reads the SA type from uninitialized memory if it is not
> > provided by the caller of sysctl(2) because of a missing length check.
> > The following patch fixes this issue.
> > 
> > diff --git sys/net/pfkeyv2.c sys/net/pfkeyv2.c
> > index a6a1648e991..c3be3616d6b 100644
> > --- sys/net/pfkeyv2.c
> > +++ sys/net/pfkeyv2.c
> > @@ -2705,7 +2705,10 @@ pfkeyv2_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp,
> >  	if (namelen < 1)
> >  		return (EINVAL);
> >  	w.w_op = name[0];
> > -	w.w_satype = name[1];
> > +	if (namelen >= 2)
> > +		w.w_satype = name[1];
> > +	else
> > +		w.w_satype = SADB_SATYPE_UNSPEC;
> >  	w.w_where = oldp;
> >  	w.w_len = oldp ? *oldlenp : 0;
> >  
> > 
> 
> I like to return EINVAL if SA type if not provided.

Diff below breaks ipsecctl(8).

# ipsecctl -sa
ipsecctl: ipsecctl_get_rules: sysctl: Invalid argument

The name[2] is also optional, so name[1] should also stay optional.
One could dislike this type of sysctl(2) interface, but it is as
it is.

OK bluhm@ for the diff above from Carsten Beckmann.

> Index: sys/net/pfkeyv2.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> diff -u -p -r1.260 pfkeyv2.c
> --- sys/net/pfkeyv2.c	11 Jan 2024 14:15:11 -0000	1.260
> +++ sys/net/pfkeyv2.c	17 May 2024 15:50:41 -0000
> @@ -2702,7 +2702,7 @@ pfkeyv2_sysctl(int *name, u_int namelen,
>  
>  	if (new)
>  		return (EPERM);
> -	if (namelen < 1)
> +	if (namelen < 2)
>  		return (EINVAL);
>  	w.w_op = name[0];
>  	w.w_satype = name[1];