that seems sensible :)
Alexander Bluhm <[email protected]> wrote:
> 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);
>
