New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Command injection vulnerabilities #447

Closed
fdcarl opened this Issue Nov 9, 2017 · 7 comments

Comments

Projects
None yet
3 participants
@fdcarl

fdcarl commented Nov 9, 2017

There are multiple command injection vulnerabilities in the current version linux-dash.

  1. The python and node versions of the servers are vulnerable to code injection. For example, with the python server running on my local host, navigating to the URL http://127.0.0.1/server/?module=;ls$IFS-al will output the listing of the current directory.

image

In the case of the node version, by using a node client commands can be executed directly. For example:

image

At this point, it is pretty trivial to gain a shell on the server. And since the readme mentions that it may require sudo, there's a good chance that shell will be a root shell.

  1. In linux_json_api.sh, the final two lines of the script are as follows:
fnCalled="$1"

${fnCalled}

Since all the various versions of the servers (go, node, php, and python) all simply pass an argument to this shell script, some limited command injection is possible. For example, any linux commands that do not need an argument. For example, when running the python version of the server on my localhost the URL http://127.0.0.1/server/?module=whoami will return:

image

Depending on the permissions of the user running the server, it may be possible to do things like DoS the system by shutting it down.

@conmarap

This comment has been minimized.

Show comment
Hide comment
@conmarap

conmarap Nov 10, 2017

Contributor

W00t?! Good find!

The plugin name at the following line

function getPluginData(pluginName, callback) {
var command = spawn(nixJsonAPIScript, [ pluginName, '' ])

pluginName needs to be escaped in some way and not piped directly to spawn.

A good solution would be to keep a list of all modules, or load them from the filesystem instead of allowing arbitrary data to be ran.

Contributor

conmarap commented Nov 10, 2017

W00t?! Good find!

The plugin name at the following line

function getPluginData(pluginName, callback) {
var command = spawn(nixJsonAPIScript, [ pluginName, '' ])

pluginName needs to be escaped in some way and not piped directly to spawn.

A good solution would be to keep a list of all modules, or load them from the filesystem instead of allowing arbitrary data to be ran.

@conmarap

This comment has been minimized.

Show comment
Hide comment
@conmarap

conmarap Nov 10, 2017

Contributor

The following lines are also potentially vulnerable to this attack:

output = subprocess.Popen(
appRootPath + modulesSubPath + " " + module,
shell = True,
stdout = subprocess.PIPE)

cmd := exec.Command("./linux_json_api.sh", module)

Since module comes directly from GET:

module := r.URL.Query().Get("module")

And that because in the bash file there's this:

fnCalled="$1"
${fnCalled}

image

And of course the PHP file...

<?php
header("Cache-Control: no-store, no-cache, must-revalidate");
header("Pragma: no-cache");
$shell_file = dirname(__FILE__) . '/linux_json_api.sh';
$module = escapeshellcmd($_GET['module']);
echo shell_exec( $shell_file . " " . $module );

Contributor

conmarap commented Nov 10, 2017

The following lines are also potentially vulnerable to this attack:

output = subprocess.Popen(
appRootPath + modulesSubPath + " " + module,
shell = True,
stdout = subprocess.PIPE)

cmd := exec.Command("./linux_json_api.sh", module)

Since module comes directly from GET:

module := r.URL.Query().Get("module")

And that because in the bash file there's this:

fnCalled="$1"
${fnCalled}

image

And of course the PHP file...

<?php
header("Cache-Control: no-store, no-cache, must-revalidate");
header("Pragma: no-cache");
$shell_file = dirname(__FILE__) . '/linux_json_api.sh';
$module = escapeshellcmd($_GET['module']);
echo shell_exec( $shell_file . " " . $module );

conmarap added a commit to conmarap/linux-dash that referenced this issue Nov 10, 2017

@MilosStanic

This comment has been minimized.

Show comment
Hide comment
@MilosStanic

MilosStanic Nov 12, 2017

Talking about security.. it would be nice to have some sort of login, also, possibly use JWT-Auth.
Thanks.

MilosStanic commented Nov 12, 2017

Talking about security.. it would be nice to have some sort of login, also, possibly use JWT-Auth.
Thanks.

@afaqurk afaqurk closed this in #448 Nov 12, 2017

afaqurk added a commit that referenced this issue Nov 12, 2017

Merge pull request #448 from conmarap/447-fix-code-injection-vuln
#447 - Make sure the module is a valid function in the bash file
@fdcarl

This comment has been minimized.

Show comment
Hide comment
@fdcarl

fdcarl Nov 13, 2017

Actually this needs to be reopened, the patch does not actually totally fix the issue. The function test can be easily bypassed to still get code execution. For example, using the python server on my local machine, http://127.0.0.1/server/modules=$(touch$IFS$9/tmp/testing) will actually execute the touch command and create the file.

fdcarl commented Nov 13, 2017

Actually this needs to be reopened, the patch does not actually totally fix the issue. The function test can be easily bypassed to still get code execution. For example, using the python server on my local machine, http://127.0.0.1/server/modules=$(touch$IFS$9/tmp/testing) will actually execute the touch command and create the file.

@conmarap

This comment has been minimized.

Show comment
Hide comment
@conmarap

conmarap Nov 13, 2017

Contributor

@fdcarl Let me evaluate and get back to you. The if statement was supposed to check if the invoked command was a function in that bash script.

Contributor

conmarap commented Nov 13, 2017

@fdcarl Let me evaluate and get back to you. The if statement was supposed to check if the invoked command was a function in that bash script.

@fdcarl

This comment has been minimized.

Show comment
Hide comment
@fdcarl

fdcarl Nov 13, 2017

So I believe the issue here is now this line:

By using the "$(...)", the command is actually being executed here which is before the new type checks.

EDIT: Nevermind, I just realized it was occurring before this line.

fdcarl commented Nov 13, 2017

So I believe the issue here is now this line:

By using the "$(...)", the command is actually being executed here which is before the new type checks.

EDIT: Nevermind, I just realized it was occurring before this line.

conmarap added a commit to conmarap/linux-dash that referenced this issue Nov 13, 2017

@conmarap

This comment has been minimized.

Show comment
Hide comment
@conmarap

conmarap Nov 13, 2017

Contributor

@fdcarl that's precise. And the same thing goes for the backquotes (actually didn't test this). Sadly this can't be mitigated in plain bash, so I cooked up escaping in Python, Go and JS.
Python and JS have been tested, but I have not tested Go as I am fuzzy with its regexp library. So, I'll do that tomorrow. I have a branch here and this commit c9d0a31. Some of it may be overkill, so I may trim where necessary.

Contributor

conmarap commented Nov 13, 2017

@fdcarl that's precise. And the same thing goes for the backquotes (actually didn't test this). Sadly this can't be mitigated in plain bash, so I cooked up escaping in Python, Go and JS.
Python and JS have been tested, but I have not tested Go as I am fuzzy with its regexp library. So, I'll do that tomorrow. I have a branch here and this commit c9d0a31. Some of it may be overkill, so I may trim where necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment