Re: more info on "radiotap: bitmap-end-finding buffer overrun"

[Henri Salo <henri () nerv fi>]

On Fri, Jan 17, 2014 at 12:50:55PM +0100, Raphael Geissert wrote:
> Hi,
>
> I was wondering if anyone has more info on the following commit:
> https://github.com/torvalds/linux/commit/bd02cd2549cfcdfc57cb5ce57ffc3feb94f70575
>
> AFAICS it is a different issue than CVE-2013-7027.
>
> A web search points to the following Secunia advisory, but not much else:
> http://secunia.com/community/advisories/56282
>
> (not asking for a CVE at this time)
>
> Cheers,
> -- 
> Raphael Geissert - Debian Developer
> www.debian.org - get.debian.net

Johannes Berg replied:

"""
It's not important at all in the current code base, let me explain why I think so.

The only (current) user of this code is the mac80211 injection code, so you
already need permission to create raw sockets, which I believe is usually
equivalent to root permissions.

With that aside, let's assume you build and manage to send a packet specifically
to hit this particular issue. By nature of the issue, this packet must consist
solely of a radiotap header, with header extension bitmap and at most 3 bytes of
data. The latter is crucial as otherwise the bitmap would just overlap the data
and you can't cause the invalid read. Now this means that your packet is really
just the size of the fixed radiotap header, plus 3 bytes at most, so at most 11
bytes.

Let's also say that the length field in your radiotap header is 8 (the minimum),
which doesn't matter for the parser but does for the surrounding code. As a
result, the checking code in ieee80211_monitor_start_xmit() will see that there
are at least 2 more bytes after the radiotap header, and treat them as the
802.11 frame control field. Regardless of the contents of those two bytes,
ieee80211_hdrlen() will return at least 10.

Since 10 + 8 (the radiotap length we put into the packet) is far bigger than 11,
I believe you can't even trigger the invalid read, since the packet will be
dropped as invalid before the real radiotap parser is even initialized (i.e. the
previously buggy code invoked.)
"""

---
Henri Salo

Attachment: signature.asc
Description: Digital signature