
Nmap Developmentmailing list archives
Re: parsing of script-args is broken
From: Patrick Donnelly <batrick () batbytes com>
Date: Mon, 27 Apr 2009 03:41:50 -0600
Hi Jah,On Sun, Apr 26, 2009 at 5:20 PM, jah <jah () zadkiel plus com> wrote:
Greetings,I haven't been able to pay as much attention lately as I would haveliked, so apologies if this has in fact been addressed despite myefforts to find any mention of the problem.--script-args vhost=domain.co.uk is, I believe a perfectly legitimateargument for a script. The intention is that the script can access thestring assigned to vhost in the nmap.registry.args table.However, run this argument through loadstring() to check that thearguments are valid lua and to return a lua table like so:loadstring( 'return {vhost=domain.co.uk}' ) and what you would get backis: attempt to index global 'domain' (a nil value)instead of seeing the string "domain.co.uk" loadstring would see a luavalue named "domain" and try to index the value with a lua value named "co".To prevent this in nse_main.lua, the script arguments string is passedthrough gsub() to place double-quotes around portions of it so thatloadfile will instead interpret those portions as strings instead ofglobal variables. Trouble is, the pattern looks for "=([%w_]+)" whichmatches alphanumeric characters and the underscore and replaces themwith "=\"%1\"". So my vhost argument becomes vhost="domain".co.ukwhich, when passed to loadstring() as'return {vhost="domain".co.uk}' results in the error: : '}' expected near '.'Hopefully it's clear that "=([%w_]+)" is insufficient to deal with allpossible script arguments which, as I understand it, could be one ormore name=value pairs where the value may be a string or a table [1].Either this method has to be made much more robust, or that replacementshould be removed entirely and force users to ensure that, whenproviding script-args, each literal string in a name=value pair is quoted:--script-args " vhost='domain.co.uk' "--script-args " vhost={'domain.co.uk','domain.com'} "as long as the arguments are valid lua and the strings are quoted, thenloadstring() will not error.I've made an attempt at improving the "stringification" of the stringpassed to loadstring() with the aim of allowing both the above examplesand those in [1]. The attached patch:replaces any commas found within quoted (on windows: a single quoteonly) string literals in the script-args with the string "Ncomma".splits the resultant string, comma delimitedquotes any trimmed, unquoted value if that value is not the key toanother value (=> foo={bar="foobar"} )joins the resultant table and replaces any "Ncomma"s with their originalcomma.So, on windows, supplying --script-args:smbuser=admin,smbpass='P455,0rd',whois={whodb=nofollow+ripe},vhost={domain.co.uk,domain.com}the following will be passed to loadstring() :smbuser="admin",smbpass='P455,0rd',whois={whodb="nofollow+ripe"},vhost={"domain.co.uk","domain.com"}which I hope covers at least as many uses as --script-args currently gets.jah[1] -http://nmap.org/book/nse-usage.html#nse-args
Your problem raises valid concerns. I've taken a more detailed lookedat some of --script-args history and technical description and here iswhat I've found:Stoiko Ivanov's thread [1] describing --script-args shows that theoriginal 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 keyswhich lead me to suggest an alternative for Kris in [2]. Stoiko's codealso disallowed '$' for unknown reasons. It also allowed an '=' signso long as it followed an initial equal sign and preceded an invalidcharacter.Last Summer, when I reviewed and rewrote much of nse_init.cc, Ichanged the parsing of --script-args to allow '$'. I believe this wasthe only significant change to the code.When nse-lua was developed I significantly shortened the amount ofcode (by writing it in Lua) needed for parsing the script-args. It isshort 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(); endendFor reasons I'm unsure of, I changed how the parsing was done to onlyinclude alphanumeric characters and an underscore. As you showed, thisis inadequate.To correct this, I have attached a small patch that correctlyimplements (most of) Stoiko's original specified functionality. I havehowever 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 peopleusing 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 ofarguments 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 quotedso we leave it untouched. Technically this violates our rules (that akey/value is allowed to contain quotation marks), but it is necessaryfor forbidden characters.So in summation, a key or value may contain any character except '{','}', ','', '=' and spaces. Values (not keys) may be surrounded byquotes (single or double) to allow all characters. A value is alsoallowed to be a nested table.[1]http://seclists.org/nmap-dev/2007/q3/0146.html[2]http://seclists.org/nmap-dev/2008/q2/0567.htmlCheers,-- -Patrick Donnelly"One of the lessons of history is that nothing is often a good thingto do and always a clever thing to say."-Will Durant
Attachment:sargs.patch
Description:
_______________________________________________Sent through the nmap-dev mailing listhttp://cgi.insecure.org/mailman/listinfo/nmap-devArchived athttp://SecLists.Org
Current thread:
- parsing of script-args is brokenjah (Apr 26)
- Re: parsing of script-args is brokenPatrick Donnelly (Apr 27)
- Re: parsing of script-args is brokenjah (Apr 27)
- Re: parsing of script-args is brokenPatrick Donnelly (Apr 27)
- patch looks goodjah (Apr 27)
- Re: parsing of script-args is brokenDavid Fifield (Apr 30)
- Re: parsing of script-args is brokenPatrick Donnelly (May 01)
- Re: parsing of script-args is brokenPatrick Donnelly (May 10)
- Re: parsing of script-args is brokenPatrick Donnelly (May 15)
- Re: parsing of script-args is brokenRon (May 17)
- Re: parsing of script-args is brokenjah (May 17)
- Re: parsing of script-args is brokenPatrick Donnelly (May 17)
- Re: parsing of script-args is brokenjah (Apr 27)
- Re: parsing of script-args is brokenPatrick Donnelly (Apr 27)