Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,3 +125,9 @@ To get around this limitation, we recommend that you declare each global variabl
/** @var \Migration $migration */
global migration;
```

### `ForbidDangerousSystemFunctionRule`

> Since GLPI 11.0.

To avoid security issues, it is recommended to not use dangerous system functions that could, for example, lead to arbitrary code execution.
1 change: 1 addition & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ services:

rules:
# declared in `services` - PHPStanGlpi\Rules\ForbidDynamicInstantiationRule
- PHPStanGlpi\Rules\ForbidDangerousSystemFunctionRule
- PHPStanGlpi\Rules\ForbidExitRule
- PHPStanGlpi\Rules\ForbidHttpResponseCodeRule
- PHPStanGlpi\Rules\MissingGlobalVarTypeRule
123 changes: 123 additions & 0 deletions src/Rules/ForbidDangerousSystemFunctionRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<?php

declare(strict_types=1);

namespace PHPStanGlpi\Rules;

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Name;
use PHPStan\Analyser\Scope;
use PHPStan\Php8StubsMap;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStanGlpi\Services\GlpiVersionResolver;

/**
* @implements Rule<FuncCall>
*/
class ForbidDangerousSystemFunctionRule implements Rule
{
private GlpiVersionResolver $glpiVersionResolver;
/** @var string[] */
private array $forbidden_functions;

public function __construct(GlpiVersionResolver $glpiVersionResolver)
{
$this->glpiVersionResolver = $glpiVersionResolver;
$this->forbidden_functions = $this->getForbiddenFunctions();
}

public function getNodeType(): string
{
return FuncCall::class;
}

public function processNode(Node $node, Scope $scope): array
{
if (\version_compare($this->glpiVersionResolver->getGlpiVersion(), '11.0.0-dev', '<')) {
// Only applies for GLPI >= 11.0.0
return [];
}

if (!$node->name instanceof Name) {
return [];
}

$function_name = $node->name->toString();
if (in_array($function_name, $this->forbidden_functions, true)) {
return [
RuleErrorBuilder::message(
"For security reason, GLPI recommends to disable the `{$function_name}` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.",
)->identifier('glpi.forbidDangerousSystemFunction')->build(),
];
}

return [];
}

/**
* @return array<string>
*/
private function getForbiddenFunctions(): array
{
// @phpstan-ignore-next-line
$stubs_map = new Php8StubsMap(PHP_VERSION_ID);
$stubs_functions = array_keys($stubs_map->functions);

/** @var array<string> $functions */
$functions = preg_grep('/^(posix_|pcntl_)/', $stubs_functions);
if (!$functions) {
throw new \RuntimeException('Regex exception');
}

// We allow some functions
$allowed_functions = [
'posix_geteuid',

'pcntl_async_signals',
'pcntl_signal',
'pcntl_signal_get_handler',
'pcntl_signal_dispatch',
];
$functions = array_diff($functions, $allowed_functions);

// Banning other functions
$functions = array_merge($functions, [
'disk_free_space',
'diskfreespace',
'dl',
'escapeshellcmd',
'exec',
'getmyuid',
'highlight_file',
'link',
'passthru',
'popen',

'proc_close',
'proc_nice',
'proc_open',
'proc_terminate',

'shell_exec',
'show_source',

// We don't use stub since we only block a few functions
'socket_accept',
'socket_bind',
'socket_clear_error',
'socket_close',
'socket_connect',
'socket_create_listen',
'socket_create_pair',
'socket_listen',
'socket_read',

'symlink',
'system',
]);

return $functions;
}
}
75 changes: 75 additions & 0 deletions tests/Rules/ForbidDangerousSystemFunctionRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);

namespace PHPStanGlpi\Tests\Rules;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use PHPStanGlpi\Rules\ForbidDangerousSystemFunctionRule;
use PHPStanGlpi\Tests\TestTrait;

/**
* @extends RuleTestCase<ForbidDangerousSystemFunctionRule>
*/
class ForbidDangerousSystemFunctionRuleTest extends RuleTestCase
{
use TestTrait;

protected function getRule(): Rule
{
return new ForbidDangerousSystemFunctionRule(
$this->getGlpiVersionResolver('11.0.0')
);
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/../data/ForbidDangerousSystemFunctionRule/test.php'], [
[
'For security reason, GLPI recommends to disable the `exec` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
6,
],
[
'For security reason, GLPI recommends to disable the `shell_exec` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
7,
],
[
'For security reason, GLPI recommends to disable the `system` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
8,
],
[
'For security reason, GLPI recommends to disable the `passthru` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
9,
],
[
'For security reason, GLPI recommends to disable the `popen` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
10,
],
[
'For security reason, GLPI recommends to disable the `proc_open` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
11,
],
[
'For security reason, GLPI recommends to disable the `pcntl_fork` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
12,
],
[
'For security reason, GLPI recommends to disable the `posix_kill` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
13,
],
[
'For security reason, GLPI recommends to disable the `dl` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
14,
],
[
'For security reason, GLPI recommends to disable the `link` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
15,
],
[
'For security reason, GLPI recommends to disable the `symlink` function. Therefore, its usage may be blocked in most GLPI instances and you should not use it.',
16,
],
]);
}
}
23 changes: 23 additions & 0 deletions tests/data/ForbidDangerousSystemFunctionRule/test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

// Forbidden functions
exec('ls');
shell_exec('ls');
system('ls');
passthru('ls');
popen('ls', 'r');
proc_open('ls', [], $pipes);
pcntl_fork();
posix_kill(1, 9);
dl('extension.so');
link('target', 'link');
symlink('target', 'link');

// Allowed functions (whitelisted)
posix_geteuid();
pcntl_async_signals(true);
pcntl_signal(SIGTERM, function() {});
pcntl_signal_get_handler(SIGTERM);
pcntl_signal_dispatch();