Re: Weird verbosity effect with nse_selectedbyname and post-scanning

[Patrick Donnelly <batrick () batbytes com>]

On Wed, Aug 11, 2010 at 10:49 AM, Djalal Harouni <tixxdz () gmail com> wrote:
> On 2010-08-10 14:04:35 -0600, David Fifield wrote:
> > Hi,
> >
> > I've just discovered that the message "NSE: Script Post-scanning.",
> > which is only supposed to be printed in verbose mode, can be printed
> > without verbose mode.
> >
> > $ ./nmap --datadir . --script http-php-version scanme.nmap.org -p80
> >
> > Starting Nmap 5.35DC18 ( http://nmap.org ) at 2010-08-10 14:00 MDT
> > Nmap scan report for scanme.nmap.org (64.13.134.52)
> > Host is up (0.066s latency).
> > PORT   STATE SERVICE
> > 80/tcp open  http
> > | http-php-version: Logo query returned unknown hash b2a24d35ffb001ed815a41578134bd46
> > |_Credits query returned unknown hash b2a24d35ffb001ed815a41578134bd46
> >
> > NSE: Script Post-scanning.
> > Nmap done: 1 IP address (1 host up) scanned in 8.94 seconds
> >
> > The secret is in the extra verbosity boost that scripts selected by name
> > get. The code calls nse_selectedbyname in nse_nmaplib.cc to see if a
> > script was selected by name, and increases the verbosity level by one if
> > it is. nse_selectedbyname calls _R[SELECTED_BY_NAME] in nse_main.lua,
> > which is
> >
> >   _R[SELECTED_BY_NAME] = function()
> >     return current and current.selected_by_name;
> >   end
> >
> > My guess is that after the normal script scanning is completed, current
> > is set to the http-php-version instance. Even though nmap.verbosity is
> > being called on behalf of the scripting engine, not a script, it still
> > gets a verbosity boost.
> >
> > Does someone have a proposed solution to this? Set current to nil after
> > a round of script scanning? It might still cause a problem when the
> > engine calls verbosity in the middle of a round. Use a separate
> > verbosity function for the engine?
> This also concerns the classic script scanning phase, if a prerule
> script is selected by name, then we'll have:
> "NSE: Script scanning 127.0.0.1"
> (host target is 127.0.0.1)
>
> I think that we should go for the simple solution, set current to nil
> after every thread:close() call. Since current is set in line 746 and
> should only be valid in the run() function (when scripts are running)
> and since it's not collected by the garbage, then we should force it
> to be nil for any *outside* verbosity call especially for the engine.
>
> The simple attached patch will clear it, and *perhaps* this will avoid
> any synchronization problems between referencing pending and running
> threads etc.

This is a good enough fix in my opinion. You only need to set it to
nil in one spot though, at the end of the for loop.

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