Re: Nmap (4.76) out-of-bounds memory access bug & patch

[Kris Katterjohn <katterjohn () gmail com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11/20/2008 07:33 PM, ithilgore.ryu.L () gmail com wrote:
> Hey everyone.
>
> I've been looking at the nmap source code lately and found a nasty
> out-of-bounds memory access bug due to insufficient validation of packet
> lengths. The problem is a bit complex so I 'll start describing it step 
> by step. Note that is applies to the latest Nmap version: 4.76. In the 
> end I 've included a patch that eliminates the bug.

Nice find!

This is my validation code, but I wasn't able to do any thorough testing since
I was in an apartment at the time (this past summer) with only one computer.

I'm curious: can you trigger this bug across the internet or just on a LAN?  I
notice you say to suppose you're on the same subnet, but don't specifically
state (that I remember) one way or another.  I only ask because routers don't
typically forward very obviously broken packets, and I'm wonder if they let
packets like this slide.

I briefly flipped through my TCP/IP Illustrateds looking for types of things
routers won't forward, but to no avail yet.  I may have been discussing this
type of thing with Brandon Enright instead of reading it somewhere, though.

> Of course, if we want to be more strict we could also reject any packet 
> that has len != iplen. But the above check is enough so that no troubles 
> are caused on our side.

Here's my comment in the code:

* Checking the IP total length (iplen) to see if its at least as large as the
* number of bytes read (len) does not work because things like the Ethernet
* CRC also get captured and are counted in len.  Therefore, after the IP total
* length is considered reasonable, iplen is used instead of len.  readip_pcap
* fixes the length on it's end after this is validated.

My proposed fix is to do any further length tests based on len instead of
iplen, even though we could get garbage data instead of actual packet.  This
is of course safer than the current way, and if for some reason a person is
generated specially crafted replies, nothing horrible will happen.

Can you try my attached patch (without comment change yet) and see if that
works for you?

> I hope my analysis was clear enough.

Very much so!  If only all bug reports (or even a small percentage) were this
detailed :)

> Cheers,
> ithilgore (sock-raw.homeunix.org)

Thanks,
Kris Katterjohn

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iQIVAwUBSSZBp/9K37xXYl36AQKlNA/6AjHvj81jM1fac+eY7t/A85vF93UFTbHB
BTZcuzBToKCL2qy15xFcX1L2JTGpHGsb4RHyq9tunaonB8C8lIGaPZEhRJWobm7u
+oOsQGkcDc68Oj7g1Gyn9oEJHOjQYbELEGj4XFFYUm0XUZHV7oEd8U4KHfBFJk84
WjGv3UL4YJRuZsHyY3ZqhscDjKOq9FgQeLd6IocDR+mmdMOJTDqOSyqvRFXSTHDn
KqamPyRx4fklqSMKG6BtUL6TiUFNR6/vhiHWSwp+qim7PKe5ob9NAYy5LE48UVz5
+hEmortF+TJE2RM6bQKID5+JMq5q8cR5osFsSRfmMzSSSYXOYxdyjehbwzEkZquG
A9MmgxEWMbLZMexIhEhVpd46vqlM4MJDAsIl29AWhSh8HneEwl+VI6gxQPOwNxzK
Nsp1Up26v+l4IzhLP8Q5hsp+QmobCtUjvPcOZz9IFf5fH8jzvD3sPBNUj/X0+ch+
4RbJrOg3iverjWNDsWkpM0O8TKIxjGDq5OMRXqO50U+gr1liD6OEl7wKmIR7e5qv
7HwW7XWo9yWBt4ckk/STa7j+vN4t+aM/41sNFE28+RhL8vipDi+WlGaAkpvh7AEq
taDSEN1O4LHpp7V0MgBo/bCnL2/zsiXBYqO3FTBsyPt0hh1/3yJB9b62QfzuM2Hw
10f8K0TXuf4=
=NeZJ
-----END PGP SIGNATURE-----

Index: tcpip.cc
===================================================================
--- tcpip.cc    (revision 11147)
+++ tcpip.cc    (working copy)
@@ -1994,19 +1994,19 @@
 
        switch (ip->ip_p) {
        case IPPROTO_TCP:
-               if (iphdrlen + sizeof(struct tcp_hdr) > iplen) {
+               if (iphdrlen + sizeof(struct tcp_hdr) > len) {
                        if (o.debugging >= 3)
                                error("Rejecting TCP packet because of incomplete header");
                        return false;
                }
-               if (!validateTCPhdr(ipc + iphdrlen, iplen - iphdrlen)) {
+               if (!validateTCPhdr(ipc + iphdrlen, len - iphdrlen)) {
                        if (o.debugging >= 3)
                                error("Rejecting TCP packet because of bad TCP header");
                        return false;
                }
                break;
        case IPPROTO_UDP:
-               if (iphdrlen + sizeof(struct udp_hdr) < iplen)
+               if (iphdrlen + sizeof(struct udp_hdr) < len)
                        break;
                if (o.debugging >= 3)
                        error("Rejecting UDP packet because of incomplete header");


_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org