On Mon, Nov 18, 2019 at 08:42:30AM +0100, Claudio Jeker wrote:
> On Sun, Nov 17, 2019 at 02:03:21PM -0700, Todd C. Miller wrote:
> > On Sun, 17 Nov 2019 20:38:59 +0100, Alexander Bluhm wrote:
> >
> > > I think the best way to handle it, is to make the kernel strict and
> > > fix userland. If the kernel would allow the sloppiest userland
> > > program to succeed, creating security would be hard.
> >
> > Sorry, I don't agree. We cannot expect userland to fill in a
> > non-standard length field. The kernel ioctl handler should set
> > sa_len appropriately instead.
>
> I agree. It should do the same check and assignment as for bind(), etc.
It is no that easy.
bind(2) uses the socklen_t namelen from the syscall args and uses
it to set the address length.
Interface address ioclt(2) for inet had no checks, but stored the
sa_len from userland and used it in some circumstances within the
kernel. That resulted in syzkaller crashes. I have implemented
strict checks, and there was no userland fallout.
For inet6 we inherited the sa_len checks from kame. SIOCDIFADDR_IN6
SIOCAIFADDR_IN6 had the sin6_len != sizeof(struct sockaddr_in6)
check forever. I changed the code to check early and make it
consistent with my inet implementation.
The get functions like SIOCGIFAFLAG_IN6 did not have a length check.
There the incoming address is used to find the correct interface
address for the requested information. This is what dhcpcd uses.
For the inet6 netmask the sin6_len is used to figure out which part
of the mask provided by userland is valid.
The diff below sets the address length for the address that dhcpcd
provides without length.
bluhm
Index: netinet6/in6.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.233
diff -u -p -r1.233 in6.c
--- netinet6/in6.c 11 Nov 2019 17:42:28 -0000 1.233
+++ netinet6/in6.c 18 Nov 2019 16:49:03 -0000
@@ -406,6 +406,7 @@ in6_ioctl_get(u_long cmd, caddr_t data,
sa = sin6tosa(&ifr->ifr_addr);
if (sa->sa_family == AF_INET6) {
+ sa->sa_len = sizeof(struct sockaddr_in6);
error = in6_sa2sin6(sa, &sa6);
if (error)
return (error);
