Re: [NSE] Adding mkdir support

[Djalal Harouni <tixxdz () opendz org>]

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
> alternative.
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
Right.

> 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.


Thanks.

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