Consider using whitelisting in input validation at /juci/lua/core.lua line 84

[ghost]

As the comment on the line 80 says, the currently used blacklist approach is dangerous. There are many other symbols (e.g. backtick, ampersand etc) and symbol sequences (e.g "$()") that can be used to perform OS command injection attacks. For example using "127.0.0.1 && /bin/cat /etc/shadow" as the host parameter value in /plugins/juci-diagnostics/rpc/diagnostics.lua causes displaying the shadow file contents.

Recommendation: consider using whitelist based approach instead of a blacklist. In case any of the arguments contain a symbol that is not in a specified whitelist, drop all arguments and raise an error.

[mkschreder]

Sounds fair. I do not like the whole idea of passing userdata to shell
commands myself. It is just so useful at times to be able to write shell
oneliners in lua scripts before processing output that this is allowed
using this shell method but I want all such things to go through this
single function where we do proper escaping of all the special symbols.

Also, this core lua library is now part of the rpc server backend here:
https://github.com/mkschreder/orangerpcd/blob/master/lualib/orange/core.lua
so I'm removing obsolete copies of these files from this repo.

As I'm adding more unit tests right now, it would be great to do proper
coverage of all possible test cases. I will be adding tests for lua part
shortly. Unit tests are currently run by travis:
https://travis-ci.org/mkschreder/orangerpcd and coverage is uploaded to
coveralls.io: https://coveralls.io/github/mkschreder/orangerpcd.

On 3 Sep 2016 11:24, "nelicacs" notifications@github.com wrote:

As the comment on the line 80 says, the currently used blacklist approach
is dangerous. There are many other symbols (e.g. backtick, ampersand etc)
and symbol sequences (e.g "$()") that can be used to perform OS command
injection attacks. For example using "127.0.0.1 && /bin/cat /etc/shadow" as
the host parameter value in /plugins/juci-diagnostics/rpc/diagnostics.lua
causes displaying the shadow file contents.

Recommendation: consider using whitelist based approach instead of a
blacklist. In case any of the arguments contain a symbol that is not in a
specified whitelist, drop all arguments and raise an error.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#58, or mute the thread
https://github.com/notifications/unsubscribe-auth/AERqiWJFpMdPptwKEniFfTn9hznMTZOxks5qmTzMgaJpZM4J0Q8j
.