Re: many scripts with output cause abort

[Brandon Enright <bmenrigh () ucsd edu>]

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

On Mon, 16 Mar 2009 17:46:10 -0600
Patrick Donnelly <batrick.donnelly () gmail com> wrote:

> Hello list,
>
> I've run into a problem that causes an ABORT for all the branches I'm
> doing performance testing on for NSE. The problem appears to be
> unrelated to NSE except having huge numbers of scripts with output
> seems to cause it. Sometime while the output table is being printed
> the code will mess the heap up. I've attached example output and the
> shell script that creates the scripts.
>
> This code should reproduce the issue:
>
> batrick:~/nmap/svn/nmap$ rm scripts/*
> batrick:~/nmap/svn/nmap$ sdb_test2.sh scripts
> batrick:~/nmap/svn/nmap$ ./nmap --script-updatedb
>
> Starting Nmap 4.85BETA4 ( http://nmap.org ) at 2009-03-16 02:16 MDT
> NSE script database updated successfully.
> Nmap done: 0 IP addresses (0 hosts up) scanned in 0.13 seconds
> batrick:~/nmap/svn/nmap$ ./nmap --script all localhost
> [...]
>
> I truncated the rest.
>
> Hopefully someone should be able to locate the issue as I am currently
> unable to investigate this.

This is the first time I've looked at this code so I still need to
figure out what is broken but so far this is what I have.  Ultimately
the problem is in NmapOutputTable::printableSize().

If you run in Valgrind you get this (line numbers are off because I
added some prints to stderr):

Entering printtableTable, tableout[(nil)] is 0
Resized tableout[0xebc9718] to size 481519 tableoutsz
Setting p to 0xebc9718
==15134==
==15134== Invalid write of size 1
==15134==    at 0x4B2D17: NmapOutputTable::printableTable(int*) (NmapOutputTable.cc:296)
==15134==    by 0x47D7CD: printportoutput(Target*, PortList*) (output.cc:862)
==15134==    by 0x451919: nmap_main(int, char**) (nmap.cc:1837)
==15134==    by 0x448E99: main (main.cc:224)
==15134==  Address 0xec3f007 is 0 bytes after a block of size 481,519 alloc'd
==15134==    at 0x4C214D0: malloc (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==15134==    by 0x4C21604: realloc (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==15134==    by 0x4D1DD2: safe_realloc (nbase_memalloc.c:137)
==15134==    by 0x4B2C4E: NmapOutputTable::printableTable(int*) (NmapOutputTable.cc:281)
==15134==    by 0x47D7CD: printportoutput(Target*, PortList*) (output.cc:862)
==15134==    by 0x451919: nmap_main(int, char**) (nmap.cc:1837)
==15134==    by 0x448E99: main (main.cc:224)
==15134==

Which when you attach GDB gives us this:

Attaching to program: /proc/26232/fd/65525, process 26232
0x00000000004b2d17 in NmapOutputTable::printableTable (this=0x75c9fb0, 
    size=0x0) at NmapOutputTable.cc:296
296           memcpy(p, cell->str,  cell->strlength);

The memcpy to put this cell in the table p is overrunning the allocated
memory.

Here are some pertinent prints:

The size that maxsz (a call to NmapOutputTable::printableSize()) thinks
the table should be:

(gdb) p maxsz
$1 = 481518

The location of the start of the table:

(gdb) p (void *)tableout
$4 = (void *) 0xebc9718

Our current pointer into the table:

(gdb) p (void *)p
$5 = (void *) 0xec3effc

The point at which our write faults is 0xec3f007 which when you do the
math comes out very nicely:

(gdb) p /x (481518 + 1 + 0xebc9718)
$7 = 0xec3f007

And for kicks, this is what we were writing at the time:
(gdb) p cell->strlength
$2 = 37
(gdb) p cell->str
$3 = 0x8993b30 "|_ test_sdb_611: some constant string"


The issue is that maxsz is not computed properly in the
NmapOutputTable::printableSize() routine.  I haven't figured out what
printtableSize() is supposed to be doing because I'm not sure I
understand what a cell is supposed to be.

If printtableSize() is doing what I think it is then in most cases it
would cause *way* to much memory to be allocated.  In this case
obviously it wasn't enough.

A better approach may be to dynamically reallocate memory in
NmapOutputTable::printableTable() whenever a cell string + space
padding is too much to fit.  If we started off at 1k and doubled the
size sent to realloc() every time we ran out we'd waste quite a bit
less memory in the general case and probably wouldn't have to worry
about making printtableSize() correct.

I'll keep looking at the code.

Brandon

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)

iEYEARECAAYFAkm+/7kACgkQqaGPzAsl94JFxgCgkdoy26XMnd0kIi5Qm5k+l3Oy
aQMAoJpT6x6er0/b4NJCYs0HPLh6IQgz
=Edn7
-----END PGP SIGNATURE-----

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