N°3412 - Command Injection vulnerability in the Setup Wizard - fix test and code

This commit is contained in:
odain
2021-02-16 17:12:41 +01:00
parent 905ee19519
commit e9cff0920b
2 changed files with 73 additions and 8 deletions

View File

@@ -551,15 +551,17 @@ class SetupUtils
SetupPage::log('Info - PHP functions disabled: '.implode(', ', $aDisabled));
if (in_array('exec', $aDisabled))
{
$aResult[] = new CheckResult(CheckResult::ERROR, "The PHP exec() function has been disabled on this server");
return new CheckResult(CheckResult::ERROR, "The PHP exec() function has been disabled on this server");
}
$sEscapedGraphvizPath = \escapeshellarg($sGraphvizPath);
if (!is_file($sEscapedGraphvizPath) || ! is_executable($sEscapedGraphvizPath)){
clearstatcache();
if (!is_file($sGraphvizPath) || ! is_executable($sGraphvizPath)){
//N°3412 avoid shell injection
return new CheckResult(CheckResult::WARNING, "$sGraphvizPath could not be executed: Please make sure it is installed and in the path");
return new CheckResult(CheckResult::ERROR, "$sGraphvizPath could not be executed: Please make sure it is installed and in the path");
}
$sGraphvizPath = escapeshellcmd($sGraphvizPath);
// availability of dot / dot.exe
if (empty($sGraphvizPath))
{
@@ -574,10 +576,6 @@ class SetupUtils
{
$oResult = new CheckResult(CheckResult::INFO, "dot is present: ".$aOutput[0]);
}
elseif ($iRetCode == 1)
{
$oResult = new CheckResult(CheckResult::WARNING, "dot could not be found: ".implode(' ', $aOutput)." - Please make sure it is installed and in the path.");
}
else
{
$oResult = new CheckResult(CheckResult::WARNING, "dot could not be executed (retcode=$iRetCode): Please make sure it is installed and in the path");

View File

@@ -0,0 +1,67 @@
<?php
namespace Combodo\iTop\Test\UnitTest\Setup;
use Combodo\iTop\Test\UnitTest\ItopTestCase;
use SetupUtils;
/**
* Class SetupUtilsTest
*
* @runTestsInSeparateProcesses
* @preserveGlobalState disabled
* @backupGlobals disabled
*
* @covers SetupUtils
*
* @since 2.7.4 N°3412
* @package Combodo\iTop\Test\UnitTest\Setup
*/
class SetupUtilsTest extends ItopTestCase
{
protected function setUp()
{
parent::setUp();
require_once APPROOT.'setup/setuputils.class.inc.php';
require_once APPROOT.'setup/setuppage.class.inc.php';
}
/**
* @dataProvider CheckGravitzProvider
*/
public function testCheckGravitz($sScriptPath, $iSeverity, $sLabel){
/** @var \CheckResult $oCheck */
$oCheck = SetupUtils::CheckGraphviz($sScriptPath);
$this->assertEquals($iSeverity, $oCheck->iSeverity);
$this->assertContains($sLabel, $oCheck->sLabel);
}
public function CheckGravitzProvider(){
if (substr(PHP_OS,0,3) === 'WIN'){
return [];
}
return [
"bash injection" => [
"touch /tmp/toto",
0,
"could not be executed: Please make sure it is installed and in the path",
],
"command ok" => [
"/usr/bin/whereis",
2,
"",
],
"command failed" => [
"/bin/ls",
1,
"dot could not be executed (retcode=2): Please make sure it is installed and in the path",
]
];
}
}