From owner-freebsd-audit  Tue Feb 13 12:26:23 2001
Delivered-To: freebsd-audit@freebsd.org
Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20])
	by hub.freebsd.org (Postfix) with ESMTP id 42F1C37B503
	for <audit@freebsd.org>; Tue, 13 Feb 2001 12:26:17 -0800 (PST)
Received: (from bright@localhost)
	by fw.wintelcom.net (8.10.0/8.10.0) id f1DKQGH15569;
	Tue, 13 Feb 2001 12:26:16 -0800 (PST)
Date: Tue, 13 Feb 2001 12:26:16 -0800
From: Alfred Perlstein <bright@wintelcom.net>
To: security@freebsd.oreg, audit@freebsd.org
Subject: (forw) robustness fix for SYSCTL_OUT
Message-ID: <20010213122616.S3274@fw.wintelcom.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
Sender: owner-freebsd-audit@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

Linux recently had a major bug through sysctl/procfs, anyone
want to take a look at this?

----- Forwarded message from Thomas Moestl <tmoestl@gmx.net> -----

From: Thomas Moestl <tmoestl@gmx.net>
To: freebsd-hackers@FreeBSD.ORG
Subject: robustness fix for SYSCTL_OUT
Date: Tue, 13 Feb 2001 21:19:02 +0100
Message-ID: <20010213211902.A873@crow.dom2ip.de>
User-Agent: Mutt/1.2.5i
Sender: owner-freebsd-hackers@FreeBSD.ORG

Hi,

the following is from sys/kern/kern_sysctl.c:

static int
sysctl_old_kernel(struct sysctl_req *req, const void *p, size_t l)
{
	size_t i = 0;

	if (req->oldptr) {
		i = l;
		if (i > req->oldlen - req->oldidx)
			i = req->oldlen - req->oldidx;
		if (i > 0)
			bcopy(p, (char *)req->oldptr + req->oldidx, i);
	}
	req->oldidx += l;
	if (req->oldptr && i != l)
		return (ENOMEM);
	return (0);
}

oldidx and oldlen are both size_t (unsigned). If l happens to be larger
than (req->oldlen - req->oldidx), ENOMEM is returned correctly, but
req->oldidx is increased by the full length. If a buggy caller does
not react on the error and calls SYSCTL_OUT again (SYSCTL_OUT normally
causes sysctl_old_kernel() or sysctl_old_user, which has a similar bug,
to be called), oldidx will be greater than oldlen, and since both are
unsigned, the if test will fail, so we will bcopy outside of the buffer
and no longer return ENOMEM.

Not that this does not matter if SYSCTL_OUT is used correctly, but for
the sake of robustness, I think it should be fixed.
Currently, there is one place in the -CURRENT kernel (that I know of)
that actually gets hit by this bug. -STABLE seems fine.

I propose the attached fix. Could it please be reviewed and commited if
correct?

	- thomas



*** sys.3/kern/kern_sysctl.c	Tue Feb 13 16:15:52 2001
--- sys/kern/kern_sysctl.c	Tue Feb 13 20:06:37 2001
***************
*** 817,824 ****
  
  	if (req->oldptr) {
  		i = l;
! 		if (i > req->oldlen - req->oldidx)
! 			i = req->oldlen - req->oldidx;
  		if (i > 0)
  			bcopy(p, (char *)req->oldptr + req->oldidx, i);
  	}
--- 817,827 ----
  
  	if (req->oldptr) {
  		i = l;
! 		if (req->oldlen <= req->oldidx)
! 			i = 0;
! 		else
! 			if (i > req->oldlen - req->oldidx)
! 				i = req->oldlen - req->oldidx;
  		if (i > 0)
  			bcopy(p, (char *)req->oldptr + req->oldidx, i);
  	}
***************
*** 914,921 ****
  	}
  	if (req->oldptr) {
  		i = l;
! 		if (i > req->oldlen - req->oldidx)
! 			i = req->oldlen - req->oldidx;
  		if (i > 0)
  			error = copyout(p, (char *)req->oldptr + req->oldidx,
  					i);
--- 917,927 ----
  	}
  	if (req->oldptr) {
  		i = l;
! 		if (req->oldlen <= req->oldidx)
! 			i = 0;
! 		else
! 			if (i > req->oldlen - req->oldidx)
! 				i = req->oldlen - req->oldidx;
  		if (i > 0)
  			error = copyout(p, (char *)req->oldptr + req->oldidx,
  					i);


----- End forwarded message -----

-- 
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
"I have the heart of a child; I keep it in a jar on my desk."


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message