>Number:         6980
>Category:       kern
>Synopsis:       [dM] getpeername length bug
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Feb 10 11:35:01 1999
>Last-Modified:
>Originator:     der Mouse
>Organization:
	Dis-
>Release:        -current as of sup 1999-02-10 AM
>Environment:
	Any with
		(a) kern/uipc_syscalls.c 1.34
		    sys/mbuf.h 1.34
	or
		(b) kern/uipc_syscalls.c 1.40
		    sys/mbuf.h 1.41
	and probably most others.
>Description:
	getpeername() mishandles its length argument.  I was seeing
	something wrong with getpeername (it was returning very large
	negative numbers (eg -242053200) in the int pointed to by its
	third argument, but only sometimes).  So I went looking.

	I still don't know what's going wrong; this PR is about
	something I happened across along the way.  Specifically, in
	kern/uipc_syscalls.c, sys_getpeername(), I see

	if (len > m->m_len)
		len = m->m_len;
	error = copyout(mtod(m, caddr_t), (caddr_t)SCARG(uap, asa), (u_int)len);

	Now, len at this point is an int variable containing what
	userland passed through the pointer third argument.  And m_len
	is of type int.  Thus, by passing a negative length, userland
	can cause the copyout call to end up with a very large third
	argument.

	This is not a problem for me, because I'm on a SPARC, and the
	SPARC implementation of copyout calls bcopy, which treats its
	length argument as signed.  But if this were not true, this
	could allow userland to spy on kernel memory it's not supposed
	to.  Looking at the SPARC copyout->bcopy, it simply traps
	faults and does the copy.  If bcopy treated its length as
	unsigned, this would return EFAULT, but in the process would
	copy a nontrivial chunk of kernel VM following the mbuf into
	userland VM.  (The len<m->m_len check designed to avoid reading
	outside the mbuf would not trip because len is negative and
	both it and m_len are signed.)  I don't think it's a big reach
	to suspect that either some port meets these conditions now or
	some new port is likely to in the future.  Skimming the m68k
	copyout code makes me think it might qualify right now.

>How-To-Repeat:
	Code inspection.

>Fix:
	Make len, in sys_getpeername(), unsigned, maybe?  It might be
	preferable to check for negative len values, since the
	interface is defined as taking a (signed) int.

					der Mouse

			       mouse@rodents.montreal.qc.ca
		     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B
>Audit-Trail:
>Unformatted: