Re: [NSE] Better Handling for Require Errors

[Fyodor <fyodor () insecure org>]

On Wed, Jun 08, 2011 at 01:11:53PM -0400, Patrick Donnelly wrote:
> > Running with -d shows a stack trace and makes it more obvious what the
> > problem is ("module 'ipops' not found"), but can we do anything about
> > this?
>
> Would artificially upping the debug level be acceptable when updating
> the database?

Since I'm not 100% sure I understand all the details, I'm going to
back up for a moment and note the initial problem...

There were scripts which required certain libraries or privileges
which were failing when run without those.  There were two main
offenders: scripts which would "require "openssl"' even though some
people compile Nmap without that library, and scripts which required
root privileges.

The initial fix for this was a case-by-case system where we basically
had a pcall template for each of those which we would paste into
scripts where necessary.  They looked like:

 if pcall(require,"openssl") then
   require("tns")
 else
   portrule = function() return false end
   action = function() end
   stdnse.print_debug( 3, "Skipping %s script because OpenSSL is missing.",
       SCRIPT_NAME)
   return;
 end

and:

  if not nmap.is_privileged() then
    if not nmap.registry['firewalk'] then
      nmap.registry['firewalk'] = {}
    end

    if nmap.registry['firewalk']['rootfail'] then
      return false
    end

    nmap.registry['firewalk']['rootfail'] = true

    if nmap.verbosity() > 0 then
      nmap.log_write("stdout", SCRIPT_NAME .. ": not running for lack of privileges")
    end

    return false
  end

Your (Patrick) patch eliminated these bloated sections by (if I
understand correctly) allowing scripts to silently cease running if a
require fails rather than printing a big backtrace and existing Nmap.
You also root a 'root' library people could 'require' if they need
root access.  This reduced the blobs above to just:

require "openssl"

and

require 'root'

That looks a lot better!  But the issue is that this silent failure
behavior can mask actual errors.  This discussion is about a case
where someone miscapitalized ipOps.lua.  That caused script-updatedb
to fail with a strange and unhelpful error message.  The errors caused
from trying to run such scripts directly is the same.  When I change
'ipOps' to 'ipops' in http-tital.nse, I get:

$ ./nmap -p80 --script scripts/http-title nmap.org

Starting Nmap 5.52.IPv6.Beta2 ( http://nmap.org ) at 2011-06-09 15:52 PDT
NSE: failed to initialize the script engine:
/home/fyodor/nmap/nse_main.lua:603: attempt to index local 'script' (a nil value)
stack traceback:
        /home/fyodor/nmap/nse_main.lua:603: in function 'get_chosen_scripts'
        /home/fyodor/nmap/nse_main.lua:1030: in main chunk
        [C]: ?

QUITTING!

I'm not sure what the error looked like before the silent require
change.  It might have been just as vague.  In both cases
(--script-udpatedb and trying to run the broken script), I get the
more informative error message (noting the failure to find ipops.lua)
in -d mode.  So if the solution is increasing debug level, we should
probably do so in general for this failed to initialize the script
engine error rather than for --script-updatedb specifically.

Cheers,
Fyodor

Reference threads:
 http://seclists.org/nmap-dev/2011/q1/1080
 http://seclists.org/nmap-dev/2011/q2/449
_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://seclists.org/nmap-dev/