Socket Bug with Patch

Christopher T. Johnson (cjohnson@netgsi.com)
Fri, 12 Jul 1996 11:26:45 -0400 (EDT)


This bug is exersised when the following style of code is used:
sd = socket(*,*,*)
bind(sd, my_addr, len_my_addr) /* optional */
connect(sd, their_addr, len_their_addr); /* EVEN with UDP */

At some point later in the code, the user does a sendto()
that looks like this:
sendto(sd, "A message to be sent", len_of_message, NULL, 0);

The "NULL, 0" construction for the to address is an accepted shorthand
to the socket layer to have it dig out the destination address from
the socket structure, originaly put there with the connect() call.

The sendto() function in net/socket.c then does the standard
test to make sure that all the parameters are good. One of these
tests is done in move_addr_to_kernel(). move_addr_to_kernel() recognizes
the NULL address pointer construction but it returns NO information
when that flag is seen, instead it exits with no-error condition AS IF
it had indeed copied the address from user to kernel space.

Sendto() then proceeds to use the kernel space copy even if it is bad.

A quick check as I am writting this message shows that sys_bind() and
sys_connect() also pass garbage to the sock->ops versions when the
user address pointer is NULL.

THe most likely fix is to have an
if (!addr) return -EINVAL;
in both sys_bind() and sys_connect().

Oh, for the kernel maintainers,
* Christopher T. Johnson: NULL addr fix on conversion of
* sys_sendto() to sock->ops->sendmsg()

Thank you for your time, sorry for the long boring writeup.

Oh, the bug still exists in linux-2.0.6

Christopher T. Johnson

--- linux-2.0.0/net/socket.c Sat May 25 14:10:11 1996
+++ linux/net/socket.c Mon Jul 1 18:30:23 1996
@@ -944,19 +944,27 @@
if (!(sock = sockfd_lookup(fd, NULL)))
return(-ENOTSOCK);

- if(len<0)
+ if(len<0) {
return -EINVAL;
+ }
err=verify_area(VERIFY_READ,buff,len);
- if(err)
- return err;
-
- if((err=move_addr_to_kernel(addr,addr_len,address))<0)
+ if(err) {
return err;
+ }
+
+ if (addr && addr_len) {
+ if((err=move_addr_to_kernel(addr,addr_len,address))<0) {
+ return err;
+ }
+ msg.msg_name=address;
+ msg.msg_namelen=addr_len;
+ } else {
+ msg.msg_name = NULL;
+ msg.msg_namelen = 0;
+ }

iov.iov_base=buff;
iov.iov_len=len;
- msg.msg_name=address;
- msg.msg_namelen=addr_len;
msg.msg_iov=&iov;
msg.msg_iovlen=1;
msg.msg_control=NULL;