Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: parsing of script-args is broken
From: jah <jah () zadkiel plus com>
Date: Mon, 27 Apr 2009 14:53:40 +0100

On 27/04/2009 10:41, Patrick Donnelly wrote:
Hi Jah,

Your problem raises valid concerns. I've taken a more detailed looked
at some of --script-args history and technical description and here is
what I've found:

Stoiko Ivanov's thread [1] describing --script-args shows that the
original functionality would allow:

   script-arguments (both key and values) may contain any characters,
   apart from '{', '}', '=' and ','

However, the original implementation neglected to allow this for keys
which lead me to suggest an alternative for Kris in [2]. Stoiko's code
also disallowed '$' for unknown reasons. It also allowed an '=' sign
so long as it followed an initial equal sign and preceded an invalid
character.

Last Summer, when I reviewed and rewrote much of nse_init.cc, I
changed the parsing of --script-args to allow '$'. I believe this was
the only significant change to the code.

When nse-lua was developed I significantly shortened the amount of
code (by writing it in Lua) needed for parsing the script-args. It is
short enough to include here:

do -- Load script arguments
  local args = gsub((cnse.scriptargs or ""), "=([%w_]+)", "=\"%1\"");
  local argsf, err = loadstring("return {"..args.."}", "Script Arguments");
  if not argsf then
    error("failed to parse --script-args:\n"..args.."\n"..err);
  else
    nmap.registry.args = argsf();
  end
end

For reasons I'm unsure of, I changed how the parsing was done to only
include alphanumeric characters and an underscore. As you showed, this
is inadequate.

To correct this, I have attached a small patch that correctly
implements (most of) Stoiko's original specified functionality. I have
however disallowed the use of spaces in any names. For example:

nmap --script-args " host = somehost "

will _not_ be interpreted as { [" host "] = " somehost " }

While that may not seem like a likely scenario, I could see people
using script-args written over multiple lines in shell scripts so,
naturally, including whitespace in names would be bad.

In my tests it works pretty well. Here is an "exhaustive" example of
arguments covering most use cases and the subsequent translation:

vhost=domain.co.uk,smbpass="P455,0rd",whois={whodb=nofollow+ripe},zone-transfer={pass=barbar}

nmap.registry.args =
{["vhost"]="domain.co.uk",["smbpass"]="P455,0rd",["whois"]={["whodb"]="nofollow+ripe"},["zone-transfer"]={["pass"]="barbar"}}

You'll notice that in smbpass="P455,0rd", the value is already quoted
so we leave it untouched. Technically this violates our rules (that a
key/value is allowed to contain quotation marks), but it is necessary
for forbidden characters.

So in summation, a key or value may contain any character except '{',
'}', ','', '=' and spaces. Values (not keys) may be surrounded by
quotes (single or double) to allow all characters. A value is also
allowed to be a nested table.

[1] http://seclists.org/nmap-dev/2007/q3/0146.html
[2] http://seclists.org/nmap-dev/2008/q2/0567.html

Cheers,
Hi Patrick,

This patch looks good to me.  Do you think we should allow lists too:
vhost={domain.co.uk,domain.com}?
Your patch will pass that example to loadstring() as:
["vhost"]={domain.co.uk,domain.com}
whereas it is intented as:
["vhost"]={"domain.co.uk", "domain.com"}
supplying the arguments with quoted list-members works fine by the way:
--script-args vhost={'domain.co.uk','domain.com'}
=> ["vhost"]={'domain.co.uk','domain.com'}

A list seems to me to be a reasonable thing to be able to supply to a
script and with this patch, the user will have to quote the list items
themselves which isn't very intuitive given that we can quote key/value
pairs on their behalf.

jah


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


  By Date           By Thread  

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