Home page logo

nmap-dev logo Nmap Development mailing list archives

Re: [NSE] Adding mkdir support
From: Djalal Harouni <tixxdz () opendz org>
Date: Wed, 4 Apr 2012 20:55:50 +0100

Hi Patrik,

On Sat, Mar 31, 2012 at 10:20:09PM +0200, Patrik Karlsson wrote:
Hi Djalal,

Your comment got me thinking and while working on the luadoc documentation
I went through all of the functionality the fs library provides. To be
honest, I don't think it should be a problem including any of the functions.
Ok, I took a quick look, and I think that we should modify the following:

make_dir() function calls mkdir() in this way:
   mkdir (path, S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP |
                S_IWGRP | S_IXGRP | S_IROTH | S_IXOTH );

At least we should use 0755 regardless of the current value of umask
and avoid situations where groups are shared...

Other solutions:
1) use 0700 mode ? (I like this one).
2) add an extra argument to control the mode ?
David, Patrick what do you think ? should we report this upstream ?

BTW while we are at this thing, IMHO we should add checks to only write
to directories owned by the current user and writable only by the current
user, and try to avoid issues that can raise from following crafted urls
and links from servers. Perhaps we should define something like "/" root
directory and write all the stuff there in case...

I don't think that we need to export all the lfs functions (I don't like
the rmdir...), just the ones we need, beside that before we include this
we should probably define how scripts will create dirs/files and where...

We already have NSE scripts creating files on the filesystem using the io
library, the additional functionality provided by the lfs library doesn't
change anything here. In regards to rmdir, I agree, it might be scary at
first look, but on the other hand, it wont remove a directory unless the
directory is empty. The lfs library does not provide a function to delete
files, and I don't see that in the io library either?
Yes and _sometimes_ these things are ok if user specifies the writable
files/dirs, but we can improve things by adding the above checks.

Just to avoid the unix tricks and to avoid situations where scripts create
lot of dirs/files according to network input ... (if a script contains a
bug...) and to keep Nmap's spirit about files.

A poorly implemented script could unfortunately endup overwriting system
files in case Nmap is being run in privileged mode, but again, this won't
change with the addition of the lfs library. To be honest, everything added
by the lfs module is actually already feasible using os.execute function.
This was unfortunately my first idea when implementing the http-mirror
script, which I very quickly realized was an extremely BAD one. In my
I've always been wondering about issues like this: pop links to a shell
and start curl,wget... and these links contain shell meta-characters ...

opinion providing this functionality through the fs module is the safer
Yes, fs module should be safer. For the os.execute() as you have mentioned
these functions should not be used by NSE scripts.

I think we should be on the watch for scripts making use of potentially
troublesome functions such as eg. fs.rmdir, os.execute, loadstring and make
sure they are thoroughly reviewed before committed. Could we trigger

something searching for this when new code is committed as an additional
check maybe?
That would be nice, a simple script that evaluates or identifies Lua
functions will help.

This can be a simple and nice GSoC task.


Sent through the nmap-dev mailing list
Archived at http://seclists.org/nmap-dev/

  By Date           By Thread  

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