Re: parsing of script-args is broken

[jah <jah () zadkiel plus com>]

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