shell_exec

[MeiRos]

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"}

[tflidd]

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.

[MeiRos]

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.

[Mikrz]

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.

[makuser]

as @kesselb already said in the original issue nextcloud/server#18659 (comment), 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.

[kesselb]

I already replaced the shell_exec call for /proc/meminfo here: #183.

I will look into:

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

[J0WI]

What about disk_free_space and disk_free_space to replace df?

serverinfo/lib/OperatingSystems/DefaultOs.php

Line 209 in 9dfa5dd

$disks = $this->executeCommand('df -TP');

[kesselb]

Could work together with /proc/mounts.

[HigH-HawK]

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

@kesselb I guess this issue can be closed :)

[J0WI]

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

serverinfo/lib/OperatingSystems/DefaultOs.php

Lines 217 to 231 in 3230b6e

	protected function readContent(string $filename): string {
	$data = @file_get_contents($filename);
	if ($data === false || $data === '') {
	throw new \RuntimeException('Unable to read: "' . $filename . '"');
	}
	return $data;
	}
	protected function executeCommand(string $command): string {
	$output = @shell_exec(escapeshellcmd($command));
	if ($output === null || $output === '') {
	throw new \RuntimeException('No output for command: "' . $command . '"');
	}
	return $output;
	}

[HigH-HawK]

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.

[J0WI]

The issue is to avoid shell_exec completely to avoid issues on systems where the function is disabled. #169 (comment) is the only remaining use of executeCommand().

[kesselb]

shell_exec is also used for the network section.

[J0WI]

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

[HigH-HawK]

@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.

[kesselb]

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.

[HigH-HawK]

That looks good 👍

[solracsf]

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

[HighPriest]

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.

[derekslenk]

Still hope people are thinking about this

[qchn]

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

[tflidd]

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.

[qchn]

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... :-)

[kesselb]

<?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?