serverinfo icon indicating copy to clipboard operation
serverinfo copied to clipboard

shell_exec

Open MeiRos opened this issue 4 years ago • 23 comments

Is it possible to not use shell_exec because it's unsafe? There are people who have disabled it and that's causing errors. Of course that also means something isn't working.

error shell_exec() has been disabled for security reasons

Steps to reproduce

1.Install NC18.0.0 RC1 with PHP 7.4.1 2.Open the NC and go to the settings/system information 3.See log

Expected behaviour

No errors

Actual behaviour

I got errors and for example I can't see network information

Server configuration

Operating system: Centos 7.8

Web server: Nginx 1.17.7

Database: MariaDB 10.3.21 PHP version: 7.4.1

Nextcloud version: Nextcloud 18.0.0 RC1

Updated from an older Nextcloud/ownCloud or fresh install: fresh

Where did you install Nextcloud from: download.nextcloud.com

Signing status:

Signing status
No errors have been found.

List of activated apps:

Only default which are tested

Are you using external storage, if yes which one: no

Are you using encryption: no

Are you using an external user-backend, if yes which one: no

Logs

Nextcloud log (data/nextcloud.log)

Nextcloud log
Insert your Nextcloud log here
{"reqId":"rgMe4fuvcWCRhoGuelTv","level":3,"time":"2020-01-04T21:14:33+00:00","remoteAddr":"x","user":"admin","app":"PHP","method":"GET","url":"/ocs/v2.php/apps/serverinfo/api/v1//basicdata?format=json","message":"shell_exec() has been disabled for security reasons at /home/nginx/home.net/public/apps/serverinfo/lib/OperatingSystems/DefaultOs.php#96","userAgent":"Mozilla/5.0 (X11; CrOS x86_64 12607.58.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.86 Safari/537.36","version":"18.0.0.8","id":"5e1101566eed7"}

{"reqId":"rgMe4fuvcWCRhoGuelTv","level":3,"time":"2020-01-04T21:14:33+00:00","remoteAddr":"x","user":"admin","app":"PHP","method":"GET","url":"/ocs/v2.php/apps/serverinfo/api/v1//basicdata?format=json","message":"shell_exec() has been disabled for security reasons at /home/nginx/home.net/public/apps/serverinfo/lib/OperatingSystems/DefaultOs.php#95","userAgent":"Mozilla/5.0 (X11; CrOS x86_64 12607.58.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.86 Safari/537.36","version":"18.0.0.8","id":"5e1101566eee8"}

{"reqId":"rgMe4fuvcWCRhoGuelTv","level":3,"time":"2020-01-04T21:14:33+00:00","remoteAddr":"x","user":"admin","app":"PHP","method":"GET","url":"/ocs/v2.php/apps/serverinfo/api/v1//basicdata?format=json","message":"shell_exec() has been disabled for security reasons at /home/nginx/home.net/public/apps/serverinfo/lib/OperatingSystems/DefaultOs.php#87","userAgent":"Mozilla/5.0 (X11; CrOS x86_64 12607.58.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.86 Safari/537.36","version":"18.0.0.8","id":"5e1101566eef5"}

{"reqId":"rgMe4fuvcWCRhoGuelTv","level":3,"time":"2020-01-04T21:14:33+00:00","remoteAddr":"x","user":"admin","app":"PHP","method":"GET","url":"/ocs/v2.php/apps/serverinfo/api/v1//basicdata?format=json","message":"shell_exec() has been disabled for security reasons at /home/nginx/home.net/public/apps/serverinfo/lib/OperatingSystems/DefaultOs.php#79","userAgent":"Mozilla/5.0 (X11; CrOS x86_64 12607.58.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.86 Safari/537.36","version":"18.0.0.8","id":"5e1101566ef01"}

MeiRos avatar Jan 06 '20 19:01 MeiRos

You mean a different way to execute shell commands? Because without shell, it's probably hard to get system information, not sure perhaps there are modules or other ways to get this.

tflidd avatar Feb 25 '20 22:02 tflidd

Errors on lines 79 and 87 are about uptime. I have read that uptime can be read with file_get_contents. I can't write code so I don't know if it's suitable here. Or is it any safer... It doesn't matter how we'll get the same information if it's done more securely it's better. (with or without shell)

If there are no other ways to do this. Is it possible to get rid of the errors in the log? Like testing if shell_exec is available and giving notification to serverinfo if not and skipping shell_exec parts in the code. Just no errors to log.

MeiRos avatar Feb 26 '20 09:02 MeiRos

Regarding Uptime calculation, for Linux systems at least you can read /proc/uptime to get the seconds since last boot as the first number in the file. There's also /proc/net/dev that can be used to determine network interface usage, at least accumulated usage. This would be perfectly secure as they're publicly readable "files" that any user on the system can read, with no sensitive information.

Mikrz avatar Mar 06 '20 05:03 Mikrz

as @kesselb already said in the original issue https://github.com/nextcloud/server/issues/18659#issuecomment-570921605, you will be able to get most if not all of the information without shell_exec.

So instead of doing cat /proc/cpuinfo, you can just do file_get_contents("/proc/cpuinfo") and do the grepping and awk in native php.

makuser avatar Mar 16 '20 12:03 makuser

I already replaced the shell_exec call for /proc/meminfo here: https://github.com/nextcloud/serverinfo/pull/183.

I will look into:

  • /proc/cpuinfo (that's almost finish)
  • /proc/uptime
  • shell_exec('hostname') can be replaced with a native php function.

kesselb avatar Mar 16 '20 12:03 kesselb

What about disk_free_space and disk_free_space to replace df? https://github.com/nextcloud/serverinfo/blob/9dfa5ddb2aabb1772b37ced95f8ca991e20f6c7f/lib/OperatingSystems/DefaultOs.php#L209

J0WI avatar May 20 '20 22:05 J0WI

Could work together with /proc/mounts.

kesselb avatar May 20 '20 22:05 kesselb

It looks like this has already been solved with the protected functions executeCommand() and readContent()

@kesselb I guess this issue can be closed :)

HigH-HawK avatar Oct 19 '20 21:10 HigH-HawK

executeCommand() and readContent() are just wrappers and still require shell_exec():

https://github.com/nextcloud/serverinfo/blob/3230b6e58f6fe46e208e1ed54f26e5beec060f0d/lib/OperatingSystems/DefaultOs.php#L217-L231

J0WI avatar Oct 19 '20 21:10 J0WI

readContent() doesn't as far as I can see, since it's only the equivalent to file_get_contents().

The shell_exec in the function is escaped, hence as secure as possible. Some UNIX systems still require the shell_exec because there's no file equivalent, to use readContent() on.

HigH-HawK avatar Oct 19 '20 21:10 HigH-HawK

The issue is to avoid shell_exec completely to avoid issues on systems where the function is disabled. https://github.com/nextcloud/serverinfo/issues/169#issuecomment-631762215 is the only remaining use of executeCommand().

J0WI avatar Oct 19 '20 22:10 J0WI

shell_exec is also used for the network section.

kesselb avatar Oct 20 '20 06:10 kesselb

shell_exec('cat ...') can also be replaced by readContent('...')

J0WI avatar Oct 21 '20 17:10 J0WI

@J0WI I see what you mean, thanks for the insight on this and I kind of agree, to keep functions as open as possible, in case someone can't use certain PHP functionality, due to restrictions.

Unless someone else is quicker changing it and creating a pull request, I'm happy to have a look at it.

HigH-HawK avatar Oct 26 '20 20:10 HigH-HawK

I'm already working on a new version of the network information parser: https://gist.github.com/kesselb/1d40c6f3c3491c8ecd59dcbb4ae5ea8d

It still needs shell_exec but for most information no read access to /sys/class/net is required anymore. Also it uses less shell_exec calls then before.

kesselb avatar Oct 26 '20 21:10 kesselb

That looks good 👍

HigH-HawK avatar Oct 26 '20 21:10 HigH-HawK

Shouldn't we also check if we can shell_exec() before even call it anywhere?

https://stackoverflow.com/questions/21581560/php-how-to-know-if-server-allows-shell-exec#answer-21581873

solracsf avatar Nov 18 '20 11:11 solracsf

I would like to dig out this issue, as I've come to find out the "System" Settings tab is inaccessible without access to root level applications like ifconfig. This makes the mentioned "System" tab throw a 500 error no matter if the shell_exec is allowed or not.

HighPriest avatar Jul 09 '21 02:07 HighPriest

Still hope people are thinking about this

derekslenk avatar Mar 23 '22 21:03 derekslenk

This Arbitrary code execution vector needs to be fixed, urgently. Not only because it is unnecessary to retreive the system information via shell_exec as explained in the comments before. This finding deserves a little more attention, it has been opened 2 years ago and the implementation of a fix is very easy. Please act, Nextcloud. :-)

Thanks Robert

qchn avatar Apr 19 '22 12:04 qchn

a fix is very easy. Please act, Nextcloud. :-)

Thanks to open source, you don't have to wait for Nextcloud, you can act yourself, pull requests are welcome.

tflidd avatar Apr 19 '22 16:04 tflidd

Thanks to open source, you don't have to wait for Nextcloud, you can act yourself, pull requests are welcome.

True that! Unfortunately, I'm not really into PHP but I'm sure someone around here is... :-)

qchn avatar Apr 19 '22 18:04 qchn

<?php

$route = file_get_contents('/proc/net/route');

$matches = [];
if (preg_match('#\t00000000\t(?<Gateway>\w+)\t#', $route, $matches) === 1) {
    $a = hexdec($matches['Gateway']);
    $b = long2ip($a);
    $c = explode('.', $b);
    $d = array_reverse($c);
    echo 'gateway: ' . implode('.', $d);
} else {
    echo 'gateway not found';
}

Hi, could you please give the above script a test and let me know if it detects your gateway properly?

kesselb avatar May 19 '23 22:05 kesselb