Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Uninitialized memory access in pfkeyv2_sysctl
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
Carsten Beckmann <carsten_beckmann@genua.de>, "tech@openbsd.org" <tech@openbsd.org>
Date:
Fri, 17 May 2024 20:42:36 +0300

Download raw body.

Thread
> On 17 May 2024, at 19:39, Alexander Bluhm <bluhm@openbsd.org> wrote:
> 
> 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.
> 

So, the SA type arg becomes optional for NET_KEY_SADB_DUMP too.
Will commit original diff today later.

>> 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];