Bug 166248 - CAN-2005-2490 sendmsg compat stack overflow
CAN-2005-2490 sendmsg compat stack overflow
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: kernel (Show other bugs)
4.0
All Linux
medium Severity high
: ---
: ---
Assigned To: Alexander Viro
Brian Brock
impact=important,reported=20050817,so...
: Security
Depends On:
Blocks: 156323
  Show dependency treegraph
 
Reported: 2005-08-18 06:35 EDT by Mark J. Cox (Product Security)
Modified: 2007-11-30 17:07 EST (History)
6 users (show)

See Also:
Fixed In Version: RHSA-2005-514
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-10-05 09:51:56 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch and comments from Al Viro (4.42 KB, patch)
2005-08-18 06:35 EDT, Mark J. Cox (Product Security)
no flags Details | Diff
RHEL3 version of patch from Dave Miller (14.19 KB, patch)
2005-08-22 03:53 EDT, Mark J. Cox (Product Security)
no flags Details | Diff
Updated patch, correcting CMSG_LEN() usage. (3.95 KB, patch)
2005-09-02 17:01 EDT, David Miller
no flags Details | Diff
Yeah, this one appears to work. Thanks. (4.24 KB, patch)
2005-09-04 03:57 EDT, David Woodhouse
no flags Details | Diff
Incremental (for easier DaveM review) patch to do same for RHEL3. (6.09 KB, patch)
2005-09-04 06:49 EDT, David Woodhouse
no flags Details | Diff
Test case for sendmsg() CMSG usage. (1.75 KB, text/x-csrc)
2005-09-05 02:34 EDT, David Miller
no flags Details
incremental to net/compat.c (617 bytes, patch)
2005-09-05 11:46 EDT, Alexander Viro
no flags Details | Diff
Final (hopefully) patch resulting from comment #32 (4.12 KB, patch)
2005-09-06 04:54 EDT, David Woodhouse
no flags Details | Diff

  None (edit)
Description Mark J. Cox (Product Security) 2005-08-18 06:35:21 EDT
Al Viro reported a flaw in sendmsg(). "When we copy 32bit ->msg_control contents
to kernel, we walk the same userland data twice without sanity checks on the
second pass.  Moreover, if original looks small enough, we end up copying to
on-stack array."

This is a kernel buffer overflow and therefore could lead to privilege escalation.

This affects 2.6 head, and from brief inspection the code in linux 2.6.9 is
similar and therefore RHEL4 is affected.

Marking as embargoed for now, needs reporting to vendor-sec and kernel@security
soon.
Comment 1 Mark J. Cox (Product Security) 2005-08-18 06:35:21 EDT
Created attachment 117865 [details]
Patch and comments from Al Viro
Comment 7 Mark J. Cox (Product Security) 2005-08-31 05:42:33 EDT
Notified security@kernel.org, vendor-sec. 
Embargo set for one week, 20050907:12
Comment 8 David Woodhouse 2005-09-02 12:37:34 EDT
The RHEL4 patch appears to cause portmap to fail on ppc64. 

recvmsg(3, {msg_name(16)={sa_family=AF_INET, sin_port=htons(963),
sin_addr=inet_addr("127.0.0.1")},
msg_iov(1)=[{"s\37\273a\0\0\0\0\0\0\0\2\0\1\206\240\0\0\0\2\0\0\0\1\0"...,
8800}], msg_controllen=24, {cmsg_len=24, cmsg_level=SOL_IP, cmsg_type=, ...},
msg_flags=0x80000000}, 0) = 56
sendmsg(3, {msg_name(16)={sa_family=AF_INET, sin_port=htons(963),
sin_addr=inet_addr("127.0.0.1")},
msg_iov(1)=[{"s\37\273a\0\0\0\1\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 28}],
msg_controllen=24, {cmsg_len=24, cmsg_level=SOL_IP, cmsg_type=, ...},
msg_flags=0x80000000}, 0) = -1 EINVAL (Invalid argument)
Comment 9 David Woodhouse 2005-09-02 16:33:17 EDT
 		tmp = ((ucmlen - CMSG_COMPAT_ALIGN(sizeof(*ucmsg))) +
 		       CMSG_ALIGN(sizeof(struct cmsghdr)));
+		if ((char *)kcmsg_base + kcmlen - (char *)kcmsg < CMSG_ALIGN(tmp))
+			goto Einval;

This is what's causing portmap to die. The test fails:

    c00000001e32bc40 + 28 - c00000001e32bc40 < 32
    tmp is 28 (24-12 + 16)

Should we really be comparing against CMSG_ALIGN(tmp) there instead of just
(tmp)? Or is it the initial calculation loop that's wrong, and should be
rounding up with CMSG_ALIGN() each time it does 'kcmlen += tmp'?

S'far too late on a Friday night to be working this out :)

Comment 13 David Miller 2005-09-02 17:01:20 EDT
Created attachment 118407 [details]
Updated patch, correcting CMSG_LEN() usage.
Comment 16 Alexander Viro 2005-09-02 17:31:23 EDT
ITYM "didn't add missing CMSG_ALIGN() in the first loop".  ACK.

What happens here is that we had another bug in the same function -
size of kernel buffer had been underestimated in the first loop
since we didn't adjust for alignment.  Another source of overflow,
that is.  Original patch didn't fix that bug, but with proper checks
for overflow in place in the second loop we started to see -EINVAL
instead of silently overflowing the damn thing.

Adding CMSG_ALIGN() in the first loop closes that one for good.  And
Comment 17 Alexander Viro 2005-09-02 17:36:50 EDT
grrr....  And we need it both in 2.4 and 2.6.  One note: I would
assign ->cmsg_len the original value of tmp, not the aligned-up
one.  _Probably_ OK either way for all existing uses, but we are
better off with length of payload remaining the same.  So in the
second loop do comparison with CMSG_ALIGN(tmp), then assign
tmp to ->cmsg_len, then align tmp up.
Comment 18 David Miller 2005-09-02 17:55:19 EDT
I think cmsg_len should be the aligned one, because that is what the
application would have padded it out to were it native.
Comment 19 Alexander Viro 2005-09-02 18:04:42 EDT
ACK, then
Comment 22 David Miller 2005-09-03 13:18:24 EDT
Actually, Al, I think I'm wrong.  We should set the cmsg_len to the
pre-aligned calculation.  I am pretty sure this is why David W. sees
that filure in sys_sendmsg().
Comment 23 David Woodhouse 2005-09-04 03:57:00 EDT
Created attachment 118435 [details]
Yeah, this one appears to work. Thanks.
Comment 24 David Woodhouse 2005-09-04 06:49:19 EDT
Created attachment 118437 [details]
Incremental (for easier DaveM review) patch to do same for RHEL3.
Comment 26 David Miller 2005-09-05 01:29:05 EDT
Looks great David, I'll take the RHEL3 side from here.
Comment 27 David Miller 2005-09-05 02:34:55 EDT
Created attachment 118445 [details]
Test case for sendmsg() CMSG usage.

This is a test program which specifically tests the code path
being modified by this bug fix.  Since this code path is used only
for 32-bit applications running under a 64-bit kernel, make sure
that the test program is compiled into a 32-bit binary for proper
testing.
Comment 28 David Woodhouse 2005-09-05 02:41:03 EDT
Test case passes on RHEL3 and RHEL4 ppc64 with the currently proposed patches.
Comment 29 Mark J. Cox (Product Security) 2005-09-05 04:17:05 EDT
sending updated patches to security@kernel.org and vendor-sec
Comment 30 Alexander Viro 2005-09-05 11:23:34 EDT
Sigh...  One more turd: in the second loop we must do the following:
1) check that we won't overflow
2) check that CMSG_ALIGN(tmp) >= CMSG_ALIGN(sizeof cmsg_header))
3) _then_ set ->cmsg_len
4) then align tmp up.

Otherwise we can still be tricked into overflow - one last header
can be written past the end of buffer, if kcmsg is at the end
of buffer (or just before it) and tmp is small enough (less than
sizeof).
Comment 31 Alexander Viro 2005-09-05 11:42:43 EDT
Actually, it's not quite that bad (due to previous checks we know
that tmp will be large enough).  However, we really want to check
for overflow before setting ->cmsg_len - otherwise we can get
one word written just past the end of buffer here.
Comment 32 Alexander Viro 2005-09-05 11:46:12 EDT
Created attachment 118475 [details]
incremental to net/compat.c
Comment 33 David Woodhouse 2005-09-06 04:54:31 EDT
Created attachment 118486 [details]
Final (hopefully) patch resulting from comment #32
Comment 35 Mark J. Cox (Product Security) 2005-09-08 05:23:55 EDT
Removing embargo, public today.
Comment 37 Red Hat Bugzilla 2005-10-05 09:51:56 EDT
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2005-514.html

Note You need to log in before you can comment on or make changes to this bug.