Home page logo
/

nmap-dev logo Nmap Development mailing list archives

Re: Ncat's Lua socket abstraction - API protection, error behavior?
From: Jacek Wielemborek <wielemborekj1 () gmail com>
Date: Sun, 1 Sep 2013 14:24:32 +0200

2013/8/29 Daniel Miller <bonsaiviking () gmail com>:
On 08/29/2013 11:05 AM, Jacek Wielemborek wrote:

Since all filter scripts share the same namespace, there's a risk that
one filter layer can break another and lead to some nasty crash.

How nasty are we talking? Aborting the whole program is nasty, but doesn't
hurt if you're just developing a new filter and did something wrong. As long
as you get a decent error message that somewhat indicates what went wrong, I
think allowing filter authors to cause crashes while developing filters is
not a bad thing.

The trick is that filters can not only crash themselves, but also each
other. It makes testing a bit harder, even more so if you take into
account how many events there are (some of them being fired relatively
rarely, for example connect() and close()). I tried to make the error
messages a bit cryptic, but it's pretty difficult to find every place
where something could break and do extra checks there.


while there could be some hacks done with
userdata, tables and such, they pointed out that there might be no
point trying to create a bullet-proof API.

An API doesn't need to be bullet-proof, just idiot-resistant. If there is a
clear and easy way to do what is needed, it doesn't matter so much if
there's a difficult and non-obvious way to break it. Security issues would
be the exception; if you're designing an API that separates privilege
levels, then you have to be more strict. In the case of Ncat filters, you
would want to be sure that an author cannot accidentally create a security
bug. Intentional security problems are different: if the filter runs every
line through os.execute(), then prevention should be a cricket bat,
liberally applied.

The current model is very open - children can access and modify all
the data of their parents (and it's possible I'll add a way to read
the access to their children as well to make it work in both
directions), access global variables and even freely interact with
other connections. That means that a security bug in any filter on any
connection could easily propagate everywhere. On the other hand, it's
the most powerful model - filters can modify their behavior in any way
you can think of. (see below for more explanation)


Perhaps it would be better do define things that just shouldn't be
done (like overwriting the registry) and assume that user can't do
them by mistake, so there's no point securing them all?

This is essentially my view. Simple things like ensuring a value cannot be
changed by simple assignment is all that should be necessary. If someone
knows enough to mess around with metatables, then they can take
responsibility for their own actions.

But even in the worst case (perhaps one of your global vars can be
overwritten, borking the whole program) does it hurt anyone?It's part of
learning the development environment and API that you're setting up. I'm
sure I could "break" NSE by doing dumb things, but then I'm just stuck with
a script that breaks for me. Nobody else will accept it, because it's
broken.Unless I broke it in a clever and new way (hacked it) that introduces
useful functionality---this is the potential upside of a "fragile" API.

Dan

Yeah, well, Lua is very open and it's probably a bit too open (and I
don't think it's a good idea to remove the "debug" library and remove
functions like rawset() to secure it). It's a bit like Ruby - call
"class Fixnum; def + value; return 5; end; end" and suddenly 2+2=5.
It's up to you how you use it and it obviously might lead to security
flaws.

AFAIK, I can't disable assignments/reassignments in the global scope
for some variables without doing some nasty hacks like setting an ugly
metamethod to "_G" special variable. If we agree it's necessary -
well, it *is* possible. The code would look like this:

setmetatable(_G, { __newindex =
  function(self, name, value)
    if name == "connections" then
      error("No way", 2)
    else
      rawset(_G, name, value)
    end
  end
})

Instead, my current solution is to replace global connections[] with a
version that doesn't allow to reassign any values in it ("read only")
- though tables inside of it are still modifiable.The original,
editable table is accessible via the registry - a place which they
won't edit by mistake. The same applies to connection_roots. I also
moved there "socket" internal value that stores the current socket
prototype.
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/


  By Date           By Thread  

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