Re: --excludefile causing reads in free()'d memory

[David Fifield <david () bamsoftware com>]

On Mon, Mar 16, 2009 at 03:28:02AM +0000, Brandon Enright wrote:
> On Mon, 9 Mar 2009 10:33:22 -0600 or thereabouts David Fifield
> > On Thu, Mar 05, 2009 at 08:19:52PM +0000, Brandon Enright wrote:
> > > While troubleshooting some other issues, I noticed that Valgrind
> > > complains with the following error when I use the --excludefile
> > > option:
> > >
> > > Valgrind seems to think that parse_expr(pc,o.af()) is causing some
> > > memory to be freed that is being read by the subsequent call to
> > > pc=strtok(NULL, "\t\n ");
> >
> > The bug is that TargetGroup::parse_expr calls strtok also and messes
> > up the global strtok state. parse_expr makes strtok look at the
> > strduped memory and load_exclude continues looking at it. This
> > creates a bug, which is that only the first specification on each
> > line of the file is honored.
> >
> > $ cat > excludes.txt <<EOF
> > 1.1.1.1 2.2.2.2
> > 3.3.3.3
> > EOF
> > $ nmap -d2 --excludefile excludes.txt
> > Loaded exclude target of: 1.1.1.1
> > Loaded exclude target of: 3.3.3.3
> >
> > This would not be hard to fix. Does anyone want to try? You just have
> > to remove strtok from one of the functions. The easiest way would
> > probably be to use strchr instead in TargetGroup::parse_expr.
>
> Thanks for tracking this down.
>
> Attached is a patch that changes the use of strtok() to strchr().  We
> really shouldn't be using strtok anywhere we can help it.  Besides
> being non-reentrant it modifies the string passed to it.  Anyone who
> doesn't realize that is in for a surprise when they try to understand
> the code.

Well done. I tested the patch and it works for me. Go ahead and commit
it. Be sure to mention that the bug made some targets in the exlucde
file be ignored, in addition to the invalid memory access.

David Fifield

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