Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: [NSE] Better Handling for Require Errors
From: Patrick Donnelly <batrick () batbytes com>
Date: Sat, 11 Jun 2011 21:58:20 -0400

On Thu, Jun 9, 2011 at 7:00 PM, Fyodor <fyodor () insecure org> wrote:
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!

Your analysis and understanding is correct.

 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!

This is actually a "bug". Script.new was designed to return nil in the
case of a silent error (whatever that may be; turns out the "silent
require" patch was the first use-case for that). I fixed this bug in
r23911.

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.

Well, if we up the debug level everywhere then we have the error
message all the time (even when we wanted it "silent"). The reason
script writers used pcall was to avoid printing error output to users
that would get confused (did the script break?). The goal of this
patch was to make that unnecessary and to obsolete the hack being used
to get around not being able to run (empty action/hostrule).

As I see it we can do one of three things:

(a) Go back to the old system.
(b) Always print an error message.
(c) Accept that require may be silenced for some border cases where we
don't want it silenced. Script authors should be testing with -d...

I'm partial to (c). I don't really like (a). I think (b) defeats the
entire point.

I'm open to suggestions...

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

  By Date           By Thread  

Current thread:
[ Nmap | Sec Tools | Mailing Lists | Site News | About/Contact | Advertising | Privacy ]