Skip to content

Commit

Permalink
Fix data directories safeness check
Browse files Browse the repository at this point in the history
  • Loading branch information
cedric-anne committed Oct 20, 2023
1 parent 1bffc15 commit d35432b
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 15 deletions.
25 changes: 12 additions & 13 deletions src/System/Requirement/DataDirectoriesProtectedPath.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,36 +82,35 @@ public function __construct(

protected function check()
{
$glpi_root_directory = realpath($this->glpi_root_directory);
$glpi_root_realpath = realpath($this->glpi_root_directory);

$missing_directories = [];
$unsafe_directories = [];

$var_root_directory = constant($this->var_root_constant);
$var_root_realpath = realpath($var_root_directory);
if (!is_dir($var_root_directory)) {
$missing_directories[$this->var_root_constant] = $var_root_directory;
} elseif (str_starts_with(realpath($var_root_directory), $glpi_root_directory)) {
} elseif (str_starts_with($var_root_realpath . DIRECTORY_SEPARATOR, $glpi_root_realpath . DIRECTORY_SEPARATOR)) {
$unsafe_directories[$this->var_root_constant] = $var_root_directory;
}

$var_root_directory = realpath($var_root_directory);

foreach ($this->directories_constants as $directory_constant) {
$directory_path = constant($directory_constant);
$directory = constant($directory_constant);
$realpath = realpath($directory);

if (!is_dir($directory_path)) {
$missing_directories[$directory_constant] = $directory_path;
if (!is_dir($directory)) {
$missing_directories[$directory_constant] = $directory;
continue;
}

$directory_path = realpath($directory_path);
if (str_starts_with($directory_path, $var_root_directory)) {
if (str_starts_with($realpath . DIRECTORY_SEPARATOR, $var_root_directory . DIRECTORY_SEPARATOR)) {
// Ignore directory as it is included in variable root path that is already tested independently.
continue;
}

if (str_starts_with(realpath($directory_path), $glpi_root_directory)) {
$unsafe_directories[$directory_constant] = $directory_path;
if (str_starts_with($realpath . DIRECTORY_SEPARATOR, $glpi_root_realpath . DIRECTORY_SEPARATOR)) {
$unsafe_directories[$directory_constant] = $directory;
}
}

Expand All @@ -129,15 +128,15 @@ protected function check()
if (count($unsafe_directories) > 0) {
$this->validation_messages[] = sprintf(
__('The following directories should be placed outside "%s":'),
$glpi_root_directory
$glpi_root_realpath
);
foreach ($unsafe_directories as $constant => $path) {
$this->validation_messages[] = sprintf('‣ "%s" ("%s")', $path, $constant);
}
}
$this->validation_messages[] = sprintf(
__('You can ignore this suggestion if your web server root directory is "%s".'),
$glpi_root_directory . DIRECTORY_SEPARATOR . 'public'
$glpi_root_realpath . DIRECTORY_SEPARATOR . 'public'
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ public function testCheckWithAnotherRootPath()
$root_path = sys_get_temp_dir();

$unsecure_var_root_constant = sprintf('GLPI_TEST_%08x', rand());
define($unsecure_var_root_constant, $root_path);
$unsecure_var_root_path = sprintf('%s/glpi_test_%08x', $root_path, rand());
$this->boolean(mkdir($unsecure_var_root_path))->isTrue();
define($unsecure_var_root_constant, $unsecure_var_root_path); // inside root dir

$this->newTestedInstance([$unsecure_var_root_constant], $unsecure_var_root_constant, $root_path);

Expand All @@ -174,9 +176,62 @@ public function testCheckWithAnotherRootPath()
->isEqualTo(
[
sprintf('The following directories should be placed outside "%s":', $root_path),
sprintf('‣ "%s" ("%s")', $root_path, $unsecure_var_root_constant),
sprintf('‣ "%s" ("%s")', $unsecure_var_root_path, $unsecure_var_root_constant),
sprintf('You can ignore this suggestion if your web server root directory is "%s/public".', $root_path),
]
);
}

public function testCheckWithPathThatIsSameAsRoot()
{
$root_path = sys_get_temp_dir();

$unsecure_var_root_constant = sprintf('GLPI_TEST_%08x', rand());
define($unsecure_var_root_constant, $root_path); // constant points to root dir itself, it cannot be safe

$this->newTestedInstance([$unsecure_var_root_constant], $unsecure_var_root_constant, $root_path);

$this->boolean($this->testedInstance->isValidated())->isEqualTo(false);
$this->array($this->testedInstance->getValidationMessages())
->isEqualTo(
[
sprintf('The following directories should be placed outside "%s":', $root_path),
sprintf('‣ "%s" ("%s")', $root_path, $unsecure_var_root_constant),
sprintf('You can ignore this suggestion if your web server root directory is "%s/public".', $root_path),
]
);
}

public function testCheckWithSameDirectoryPrefix()
{
$tmp_dir = sys_get_temp_dir();

$root_path = sprintf('%s/glpi_root_%08x', $tmp_dir, rand());
$this->boolean(mkdir($root_path))->isTrue();

$secure_var_root_constant = sprintf('GLPI_TEST_%08x', rand());
// generate a directory on same level starting with root path (e.g. `/var/www/glpi_files` when root is `/var/www/glpi`)
$secure_var_root_path = sprintf('%s_%08x', $root_path, rand());
$this->boolean(mkdir($secure_var_root_path))->isTrue();
define($secure_var_root_constant, $secure_var_root_path);

$secure_dir_constant1 = sprintf('GLPI_TEST_%08x', rand());
$this->boolean(mkdir($secure_var_root_path . '/_cache'))->isTrue();
define($secure_dir_constant1, $secure_var_root_path . '/_cache'); // Inside var root

$secure_dir_constant2 = sprintf('GLPI_TEST_%08x', rand());
$secure_dir_path2 = sprintf('%s/glpi_test_%08x', $tmp_dir, rand());
$this->boolean(mkdir($secure_dir_path2))->isTrue();
define($secure_dir_constant2, $secure_dir_path2); // Outside var root

$this->newTestedInstance([$secure_dir_constant1, $secure_dir_constant2], $secure_var_root_constant, $root_path);

$this->boolean($this->testedInstance->isValidated())->isEqualTo(true);
$this->array($this->testedInstance->getValidationMessages())
->isEqualTo(
[
'GLPI data directories are located in a secured path.',
]
);
}
}

0 comments on commit d35432b

Please sign in to comment.