Re: [nmap-svn] r32398 - in nmap: ncat nsock/src

[David Fifield <david () bamsoftware com>]

On Fri, Sep 27, 2013 at 10:16:15AM +0200, Henri Doreau wrote:
> 2013/9/26  <commit-mailer () nmap org>:
> > Author: david
> > Date: Thu Sep 26 07:17:08 2013
> > New Revision: 32398
> >
> > Log:
> > Use tempfile in place of tempnam.
> >
> > To avoid new GCC warnings about tempnam:
> > ncat_connect.c:789: warning: the use of `tempnam' is dangerous, better use `mkstemp'
> >
> > Doing things this way has the same race condition as tempnam did,
> > because we are unlinking the file before binding it. (The race window is
> > smaller now.) The file must not exist before binding the Unix socket, or
> > else you get an "address already in use" error. Unlinking before binding
> > is the same thing that netcat-openbsd does. See this earlier thread:
> > http://seclists.org/nmap-dev/2012/q4/336.
> [...]
> > -  nsock_log_info(ms, "Binding to %s (IOD #%li)", get_localaddr_string(iod), iod->id);
> > +  path = get_localaddr_string(iod);
> > +  unlink(path);
> > +  nsock_log_info(ms, "Binding to %s (IOD #%li)", path, iod->id);
> >    rc = bind(iod->sd, (struct sockaddr *)&iod->local, (int) iod->locallen);
>
> Hi David,
>
> I have a couple comments on this patch too.
>
> Calling unlink() unconditionally seems wrong to me, we should only do
> it when dealing with unix sockets. I wouldn't like nmap to delete my
> files just because they're called "127.0.0.1:8080" or something...

You're right. I misunderstood the context in which mksock_bind_addr is
called. I reverted the change until we can improve it.

David Fifield
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/