Always load chisels from CWD?

[srenatus]

Hi there.

The following, while slightly contrived, IMHO still points at an issue:

$ cat > fail.lua << EOF
> print(os.execute("whoami"))
> EOF
$ sysdig -cl | head -2
vagrant
0

Just luaL_loadfile()ing all *.lua files in the current working directory strikes me as an odd (and possibly dangerous) default, especially when sysdig might be run using sudo (see here) It's actually quite analogous to the "don't put . in $PATH" advice.

Is there something in place to remedy this behaviour? Does Sysdig care for, say, LUA_PATH, or might introducing a new $PATH-like environment variable be a better way to go? Like CHISELPATH, where you could also control the precedence of directories to look into for finding chisels etc.

Thanks for your work on this handy tool. :)

[ldegio]

sysdig doesn't currently use LUA_PATH, and the chisel paths are hardwired to specific directories (see https://github.com/draios/sysdig/blob/dev/userspace/libsinsp/utils.cpp#L40 and https://github.com/draios/sysdig/blob/dev/userspace/sysdig/sysdig.cpp#L283).

The way I propose to solve this is: 1) remove ./ from the hardwired chisel search path 2) add support for the CHISELPATH env variable that you suggest, with support for a special value defaultpaths, which means "use the hardwired chisel paths", and can combined with other path values. 3) If CHISELPATH is not define, sysdig defaults to defaultpaths

Is that going to be satisfactory?

[srenatus]

Hi @ldegio, thanks for your support.

Sounds great, just some minor nitpicks:

Regarding (2), I'd refrain from magic values ("defaultpaths")... but then again would this feature be needed? I don't think PATH, LUA_PATH, PYTHONPATH or any of the other *PATHs support something like this. You could just add a note regarding the default value (when CHISELPATH is unset) to the usage output or man page, and let the user decide how to set up her CHISELPATH to work with the default locations.

Plus I noticed (from your links, thanks for the pointers), that there already is a SYSDIG_CHISEL_DIR env var that might complicate things if it stayed there (for backwards compat).

What do you think about that? :)

[luca3m]

About CHISELPATH, we already have SYSDIG_CHISEL_DIR so it's not necessary. About CWD instead, what do you thing about removing . and keeping ./chisels and ~/chisels. Should it provide a good tradeoff between security and easy to use?

[srenatus]

@luca3m, thanks, that' would be great. :)

[luca3m]

Thanks @srenatus , fixed on dev branch!