NetBSD Problem Report #44054

From www@NetBSD.org  Sat Nov  6 11:21:40 2010
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id D590063BAC6
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  6 Nov 2010 11:21:40 +0000 (UTC)
Message-Id: <20101106112140.6B38563BAB2@www.NetBSD.org>
Date: Sat,  6 Nov 2010 11:21:40 +0000 (UTC)
From: o.vd.linden@quicknet.nl
Reply-To: o.vd.linden@quicknet.nl
To: gnats-bugs@NetBSD.org
Subject: Stacksmashing in handling of ioctl OOSIO* parameter
X-Send-Pr-Version: www-1.0

>Number:         44054
>Category:       kern
>Synopsis:       Stacksmashing in handling of ioctl OOSIO* parameter
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Nov 06 11:25:00 +0000 2010
>Closed-Date:    
>Last-Modified:  Tue May 12 19:20:59 +0000 2015
>Originator:     Onno van der Linden
>Release:        NetBSD/i386 5.99.39
>Organization:
>Environment:
NetBSD sheep 5.99.39 NetBSD 5.99.39 (SHEEP) #35: Sat Nov  6 11:40:11 MET 2010  onno@sheep:/usr/src/sys/arch/i386/compile/SHEEP i386
>Description:
For ioctl parameters OOSIOCGIFADDR, OOSIOCGIFDSTADDR, OOSIOCGIFBRDADDR and OOSIOCGIFNETMASK a  wrongly sized data buffer is passed on the stack to
ifreq_setaddr() where the call to memset() will smash the stack.
>How-To-Repeat:
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <netinet/in.h>

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

struct oifreq {
        char    ifr_name[IFNAMSIZ];             /* if name, e.g. "en0" */
        union {
                struct  sockaddr ifru_addr;
                struct  sockaddr ifru_dstaddr;
                struct  sockaddr ifru_broadaddr;
                short   ifru_flags;  
                int     ifru_metric;
                int     ifru_mtu; 
                int     ifru_dlt;
                u_int   ifru_value;
                void *  ifru_data;
                struct {
                        uint32_t        b_buflen;
                        void            *b_buf;
                } ifru_b;
        } ifr_ifru;
};      


#define OOSIOCGIFBRDADDR _IOWR('i', 18, struct oifreq)

main()
{
        int     fd;
        struct oifreq   ifreq;
        struct sockaddr_in      *sin;

        memset(&ifreq,'\0',sizeof ifreq);
        strcpy(ifreq.ifr_name,"wm0");

        fd = socket(AF_INET, SOCK_DGRAM, 0);
        if (fd < 0) {
                perror("socket");
                exit(1);
        }

        if (ioctl(fd, OOSIOCGIFBRDADDR, &ifreq) < 0) {
                perror("OOSIOCGIFBRDADDR");
                exit(1);
        }
        sin = (struct sockaddr_in *)&ifreq.ifr_broadaddr;
        printf("broadcast: %s\n", inet_ntoa(sin->sin_addr));

        close(fd);
}

>Fix:
compat_cvtcmd() doesn't translate the OOSIO parameter into its SIO counterpart,
there's some special handling in compat_ifioctl() already which could be used
to pass a rightly sized buffer, like the way the OSIO parameters are handled in ifioctl().

Another possible fix is to put a case statement in compat_cvtcmd() in place of the
ncmd assignment where the OOSIO parameters (and others ?) are handled as
seperate cases and in which the default case gets the ncmd assignment. But 
that probably needs some additional changes to compat_ifioctl() too.



>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44054 CVS commit: src/sys/net
Date: Sat, 6 Nov 2010 13:17:14 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Sat Nov  6 17:17:13 UTC 2010

 Modified Files:
 	src/sys/net: if.c

 Log Message:
 PR/44054: Onno van der Linden: Stacksmashing in handling of ioctl OOSIO*
 parameter.


 To generate a diff of this commit:
 cvs rdiff -u -r1.246 -r1.247 src/sys/net/if.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: kern/44054: Stacksmashing in handling of ioctl OOSIO* parameter
Date: Sat, 6 Nov 2010 13:22:27 -0400

 On Nov 6, 11:25am, o.vd.linden@quicknet.nl (o.vd.linden@quicknet.nl) wrote:
 -- Subject: kern/44054: Stacksmashing in handling of ioctl OOSIO* parameter

 Fixed in the kernel, and here's the corrected test code for reference.

 christos

 #include <sys/socket.h>
 #include <sys/ioctl.h>
 #include <net/if.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>

 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <err.h>

 struct oifreq {
         char    ifr_name[IFNAMSIZ];             /* if name, e.g. "en0" */
         union {
                 struct  sockaddr ifru_addr;
                 struct  sockaddr ifru_dstaddr;
                 struct  sockaddr ifru_broadaddr;
                 short   ifru_flags;  
                 int     ifru_metric;
                 int     ifru_mtu; 
                 int     ifru_dlt;
                 u_int   ifru_value;
                 void *  ifru_data;
                 struct {
                         uint32_t        b_buflen;
                         void            *b_buf;
                 } ifru_b;
         } ifr_ifru;
 };      


 #define OOSIOCGIFBRDADDR _IOWR('i', 18, struct oifreq)

 int
 main(void)
 {
         int     fd;
         struct oifreq   ifreq;
         struct sockaddr_in      *sin;

         memset(&ifreq, '\0', sizeof ifreq);
         strcpy(ifreq.ifr_name, "sk0");

         fd = socket(AF_INET, SOCK_DGRAM, 0);
         if (fd == -1)
                 err(1, "socket");

         sin = (struct sockaddr_in *)&ifreq.ifr_broadaddr;
 	sin->sin_family = AF_INET;
 	sin->sin_len = sizeof(*sin);
         if (ioctl(fd, OOSIOCGIFBRDADDR, &ifreq) == -1)
                 err(1, "OOSIOCGIFBRDADDR");
         printf("broadcast: %s\n", inet_ntoa(sin->sin_addr));

         close(fd);
 	return 0;
 }

State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Sun, 07 Nov 2010 00:16:11 +0000
State-Changed-Why:
christos committed a fix, does it work for you?


From: Onno van der Linden <o.vd.linden@quicknet.nl>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/44054 (Stacksmashing in handling of ioctl OOSIO*
	parameter)
Date: Sun, 7 Nov 2010 12:48:26 +0100

 On Sun, Nov 07, 2010 at 12:16:12AM +0000, wiz@NetBSD.org wrote:
 > Synopsis: Stacksmashing in handling of ioctl OOSIO* parameter
 > 
 > State-Changed-From-To: open->feedback
 > State-Changed-By: wiz@NetBSD.org
 > State-Changed-When: Sun, 07 Nov 2010 00:16:11 +0000
 > State-Changed-Why:
 > christos committed a fix, does it work for you?

 Works fine now.


 Onno

From: "Antti Kantee" <pooka@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44054 CVS commit: src/tests/net
Date: Sun, 7 Nov 2010 19:53:42 +0000

 Module Name:	src
 Committed By:	pooka
 Date:		Sun Nov  7 19:53:42 UTC 2010

 Modified Files:
 	src/tests/net: Makefile
 Added Files:
 	src/tests/net/if: Makefile t_compat.c

 Log Message:
 convert program in PR kern/44054 to an atf test case


 To generate a diff of this commit:
 cvs rdiff -u -r1.6 -r1.7 src/tests/net/Makefile
 cvs rdiff -u -r0 -r1.1 src/tests/net/if/Makefile src/tests/net/if/t_compat.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: feedback->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Mon, 08 Nov 2010 00:20:02 +0000
State-Changed-Why:
Confirmed fixed, and test case added.
Thanks!


From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44054 CVS commit: [netbsd-5] src/sys
Date: Wed, 16 Feb 2011 20:37:47 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Wed Feb 16 20:37:47 UTC 2011

 Modified Files:
 	src/sys/compat/common [netbsd-5]: uipc_syscalls_43.c
 	src/sys/net [netbsd-5]: if.c

 Log Message:
 Pull up following revision(s) (requested by chs in ticket #1541):
 	sys/compat/common/if_43.c: revision 1.3 via patch
 	sys/net/if.c: revision 1.247 via patch
 PR/44054: Onno van der Linden: Stacksmashing in handling of ioctl OOSIO*
 parameter.
 can't map the old and the new SIO calls the way we did before because the
 numbers have changed. Instead provide a switch. Keep the old code there,
 to handle cases we did not handle in the first switch, but this is a hack
 and should be removed.


 To generate a diff of this commit:
 cvs rdiff -u -r1.43 -r1.43.10.1 src/sys/compat/common/uipc_syscalls_43.c
 cvs rdiff -u -r1.230.4.3 -r1.230.4.4 src/sys/net/if.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Stephen Borrill" <sborrill@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44054 CVS commit: [netbsd-5] src/sys/net
Date: Thu, 5 Jan 2012 09:47:39 +0000

 Module Name:	src
 Committed By:	sborrill
 Date:		Thu Jan  5 09:47:39 UTC 2012

 Modified Files:
 	src/sys/net [netbsd-5]: if.c

 Log Message:
 Pull up the following revisions(s) (requested by obache in ticket #1708):
 	sys/net/if.c:	revision 1.246

 PR/44030: ifreqn2o gets called with the parameters the wrong way around.
 Reverts fix for PR 42585 (ticket #1416) as the root cause of the crash is
 addressed by PR 44054 (pullup #1541).


 To generate a diff of this commit:
 cvs rdiff -u -r1.230.4.5 -r1.230.4.6 src/sys/net/if.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: closed->open
State-Changed-By: martin@NetBSD.org
State-Changed-When: Tue, 12 May 2015 19:20:59 +0000
State-Changed-Why:
There is still a problem in kernels not using the compat code


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.