Re: [nmap-svn] r32366 - nmap-exp/d33tah/luaexec-lookup/ncat

[Jacek Wielemborek <wielemborekj1 () gmail com>]

2013/9/20 David Fifield <david () bamsoftware com>:
> On Fri, Sep 20, 2013 at 05:10:29PM +0200, Jacek Wielemborek wrote:
> > 2013/9/20  <commit-mailer () nmap org>:
> > > +static int try_open(char *filename)
> > > +{
> > > +    FILE *opened = fopen(filename, "r");
> > > +    if (opened == NULL) {
> > > +        return 0;
> > > +    } else {
> > > +        fclose(opened);
> > > +        return 1;
> > > +    }
> > > +}
> > > +
> > > +/* Try to find path for the script, looking in both the current directory and
> > > +   - if the lookup filed - in a directory relative to the Ncat's executable.
> > > +   Returns a dynamically allocated variable. */
> > > +static char* find_script_path(char *filename, char *argv0)
> > > +{
> > > +    char *exe_path, *script_path;
> > > +    int script_path_len;
> > > +
> > > +    if (try_open(filename))
> > > +        return filename;
> > > ...
> > > +    if (!try_open(script_path))
> > > +        bye("Could not find the script file in any of the search directories.");
> > > +
> > > +    return script_path;
> > > +}
> > > ...
> > > -                o.cmdexec = argv[2];
> > > +                o.cmdexec = find_script_path(argv[2], argv[0]);
> > >                  lua_setup();
> > >                  lua_run();
> >
> > You said:
> >
> > Now, I want you to split up lua_setup, into one function that just
> > initializes the lua_State, and another function where you give it a
> > script name. It's not good to have so much application logic (trying
> > multiple platform-specfic paths) in a function like lua_setup. Also, I
> > think you would be able to do this within main, which would mean you
> > don't have to push argv0 into the global scope. What you want is a
> > helper function in ncat_main.c, that first calls lua_setup, then calls
> > some other function with a name that you give it. Then you call this
> > helper function from main(). The idea is to keep these application
> > details close to the application and farther from the Lua library
> > implementation.
> >
> > What do you think about the approach I just took?
>
> I have an idea on how to improve it. As written, there is a race
> condition between when try_open checks if a file exists and lua_setup
> calls luaL_loadfile on it. You really don't want to be opening the file
> twice: either luaL_loadfile can read it, or it can't.
>
> Instead, let lua_setup do only this:
>     void lua_setup(void)
>     {
>         ncat_assert(o.cmdexec != NULL);
>
>         L = luaL_newstate();
>         luaL_openlibs(L);
>
>         /* install the traceback function */
>         last_function_number = lua_gettop(L);
>         lua_pushcfunction(L, traceback);
>         lua_insert(L, last_function_number);
>     }
> Add a new function, ncat_lua_loadfile(lua_State *, const char *), that
> does luaL_loadfile and returns success or failure. Then, in ncat_main.c,
> add a new function ncat_lua_searchfile, that calls ncat_lua_loadfile on
> different paths until it succeeds, or finally returns failure.
>
> David Fifield
> _______________________________________________
> Sent through the dev mailing list
> http://nmap.org/mailman/listinfo/dev
> Archived at http://seclists.org/nmap-dev/

Please see r32368. It somehow felt awkward.
_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/