Re: Boolean Operators for --script (again)

[David Fifield <david () bamsoftware com>]

On Sat, Apr 11, 2009 at 03:13:21AM -0600, Patrick Donnelly wrote:
> Here's an updated draft of the Augmented BNF syntax for the boolean
> --script rules to allow parenthesis to enforce operator associations.
> I've also attached an updated patch (hopefully the last one).

So, I've looked into the patch and I think I see how it works. Each rule
string is textually transformed into a Lua expression. For example,
        $ nmap --script="all and not telnet-*" localhost -sP
results in the expression
        __["all"] and not __["telnet-*"]
For each entry in script.db, the __ table has the script's category
names set to true, for example
        __ = {["all"] = true, ["default"] = true, ["safe"] = true, ...}
The __ table has a metatable that does wildcard matching on any string
that doesn't match one of the category names.

The expression generated is loaded as a Lua program and executed,
returning either true or false. So it's not a coincidence that the
grammar you gave is basically a subset of the Lua grammar.
http://www.lua.org/manual/5.1/manual.html#8

> ruleset = rule *("," rule)
> rule    = exp / ("(" exp ")")
> exp     = "nil" / "false" / "true" / name / rule binop rule / unop rule
> binop   = "and" / "or"
> unop    = "not"
> name    = %x21-%x27 /
>           %x2A-%x2B /
>           %x2D-%x3A /
>           %x3C-%x7E ; All visible characteres except ',', '(', ')', and ';'

I don't have a problem with the implementation method of converting to a
Lua string and executing it. It gets us "and", "or", and "not" for free.
This works:
        nmap --script="all and not (telnet-brute or sql-injection)"
I noticed some corner cases where the code behaves unexpectedly and I
have some suggestions.

First, why allow nil, false, and true?

The new code requires a new format for script.db (you should have
mentioned that). Currently an entry looks like
        Entry { category = "discovery", filename = "banner.nse" }
        Entry { category = "safe", filename = "banner.nse" }
The patch changes it to
        Entry {
                filename = "banner.nse",
                categories = {
                        "discovery",
                        "safe",
                }
        }
I can understand why you did that, because you want to have all the
categories at once for each entry as you iterate through script.db. But
you can get the same effect without changing the format by preprocessing
script.db into a table in the form you want, and not doing all the work
in the Entry callback.

A side effect of having Lua execute the string is that we get Lua syntax
errors:
        $ nmap --script="a and and b" localhost -sP
        NSE: failed to initialize the script engine:
        ./nse_main.lua:355: [string "return __["a"] and and __["b"]"]:1: unexpected symbol near 'and'
Do you think we should pcall it and give a higher-level error message?

Boolean expressions make trouble for our blacklisting of the "version"
category.
        nmap --script="safe,version"
        ./nse_main.lua:264: explicitly specifying rule 'version' is prohibited
while
        nmap --script="safe or version"
runs without complaint. What do you think we should do about that? I
don't really know why it's blacklisted anyway. I mean, you can always
list a "version" script by file name and it's not the end of the world.

Checking for duplicate files got changed:
        nmap --script="telnet-brute,telnet-brute.nse"
runs two copies to telnet-brute.nse.

I attached a patch that addresses some of these issues. The main change
as I see it is using the Entry function to preprocess script.db into a
nice table, allowing better factoring of the script selection code and
not requiring a new script.db. Other changes:
 * Removes "true", "false", and "nil".
 * The code acts like the "version" category doesn't exist if -sV isn't
   given. That prevents any version script from being matched but makes
   the error message worse: "'version' did not match a category,
   filename, or directory".
 * Wildcard matches are anchored at the beginning and end, so that "a"
   doesn't match "asn-query.nse".
 * Instead of converting 'a' to '__["a"]', converts to 'm("a")'. The m
   function does matching of categories and wildcard names. I think it's
   clearer than using a metatable.

Thanks for working on this. Your idea for how to implement it was a good
one.

David Fifield

Attachment: bool5.patch
Description:

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