NSE Libraries and Global Accesses (original) (raw)

nmap-dev logo

Nmap Developmentmailing list archives

From: Patrick Donnelly <batrick () batbytes com>
Date: Thu, 9 Jul 2009 02:34:20 -0600

NSE Libraries have had a difficult time lately with inappropriate (and probably accidental) use of global variables. Besides being much slower than local access, global variables (within a library) are shared by all scripts. That is, two or more scripts running some function in a library may be setting/using globals simultaneously. I of course use "simultaneously" to mean that while one script is yielded on some socket operation, another script may be changing the variables the yielded script intends to use again. To give a concrete example, we recently had this problem with the nselib/comm.lua library. The get_banner function would save a socket variable to the global "socket" variable:

get_banner = function(host, port, opts) opts = initopts(opts) opts.recv_before = true socket, nothing, correct, banner = tryssl(host, port, "", opts)

When multiple scripts use this get_banner function, each ends up overwriting the socket being used by the previous script. This is a serious problem and many other libraries had similar mistakes. The patch in this case is pretty simple:

That patch was applied in r13963. Other libraries were corrected in r14080.

These globals variables were found using a fairly simple command: luac -l -p library.lua | awk '{ if ($1 == "function") exit; print $0; }' | grep SETGLOBAL

That only catches one aspect of the problem, setting globals. That is to say, whenever a script sets a global outside the file scope (in a function). Naturally the above example sets the "socket" global. We also want to catch cases where a script uses a global that does not exist. In Lua, we say that we "index" a global. Often we hear we index a hash table; remember that global variables are actually key/value pairs in a table, the global table.

What makes a global qualify as "not existing"? Well, we consider all Lua globals (e.g. print), Nmap libraries (e.g., "nmap"), and nselib libraries (e.g. "http") as legitimate globals (they exist and are readily available). Next, we also consider the globals set by the file (e.g., http.request) as valid globals to be indexed within any function in that library. By that I mean, only the http library can index the "request" global. The http.get function, for example, uses the request function in this way:

get = function( host, port, path, options ) options = options or {} -- ... return request( host, port, data, mod_options ) end

In order to catch cases where a script indexes a global that does not (or should not) exist, I have created a bash script which will locate these problems (attached). Here is example output from before patch r14080.

batrick@batbytes:~/nmap/svn/nmap$ ./check_globals | less Checking nselib/datafiles.lua for bad global accesses Found set global,'_', at line number 186.

...

Checking nselib/imap.lua for bad global accesses Found set global,'line', at line number 24. Found set global,'status', at line number 24. Found indexed global,'line', at line number 25. Found set global,'line', at line number 28. Found set global,'status', at line number 28. Found indexed global,'status', at line number 29. Found indexed global,'status', at line number 32. Found indexed global,'line', at line number 33. Found indexed global,'line', at line number 34. Found set global,'line', at line number 34. Found indexed global,'line', at line number 35. Found set global,'line', at line number 40. Found set global,'status', at line number 40.

...

That is only a small excerpt of the actual output; as I said earlier, the problem was widespread.

I hope developers in the future will avail themselves use of this script so we no longer encounter this problem in the future. (Locals are your friend!)

Also, before anyone asks, this problem does not concern scripts outside of libraries because each script instantiation has its own unique global table; however, I would still encourage using locals instead due to performance considerations.

-- -Patrick Donnelly

"Let all men know thee, but no man know thee thoroughly: Men freely ford that see the shallows."

Attachment:check_globals
Description:

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

Current thread: