Skip to content

Commit a2536e5

Browse files
committed
Process class constructor accepts arguments as array
Previously it accepts just whole command without escaping, so it was potential unsafe code
1 parent d60e45d commit a2536e5

File tree

6 files changed

+33
-16
lines changed

6 files changed

+33
-16
lines changed

src/Process/GitBlameProcess.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,17 @@ class GitBlameProcess extends Process
99
* @param string $gitExecutable
1010
* @param string $file
1111
* @param int $line
12+
* @throws RunTimeException
1213
*/
1314
public function __construct($gitExecutable, $file, $line)
1415
{
15-
$cmd = escapeshellcmd($gitExecutable) . " blame -p -L $line,+1 " . escapeshellarg($file);
16-
parent::__construct($cmd);
16+
$arguments = array('blame', '-p', '-L', "$line,+1", $file);
17+
parent::__construct($gitExecutable, $arguments);
1718
}
1819

1920
/**
2021
* @return bool
22+
* @throws RunTimeException
2123
*/
2224
public function isSuccess()
2325
{
@@ -106,10 +108,11 @@ public function getSummary()
106108
/**
107109
* @param string $gitExecutable
108110
* @return bool
111+
* @throws RunTimeException
109112
*/
110113
public static function gitExists($gitExecutable)
111114
{
112-
$process = new Process(escapeshellcmd($gitExecutable) . ' --version');
115+
$process = new Process($gitExecutable, array('--version'));
113116
$process->waitForFinish();
114117
return $process->getStatusCode() === 0;
115118
}
@@ -120,6 +123,7 @@ public static function gitExists($gitExecutable)
120123
* @param int $time
121124
* @param string $zone
122125
* @return \DateTime
126+
* @throws \Exception
123127
*/
124128
protected function getDateTime($time, $zone)
125129
{
@@ -140,4 +144,4 @@ protected function getDateTime($time, $zone)
140144

141145
return new \DateTime($datetime->format('Y-m-d\TH:i:s') . $zone, $utcTimeZone);
142146
}
143-
}
147+
}

src/Process/LintProcess.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class LintProcess extends PhpProcess
2020
* @param bool $aspTags
2121
* @param bool $shortTag
2222
* @param bool $deprecated
23+
* @throws RunTimeException
2324
*/
2425
public function __construct(PhpExecutable $phpExecutable, $fileToCheck, $aspTags = false, $shortTag = false, $deprecated = false)
2526
{
@@ -33,7 +34,7 @@ public function __construct(PhpExecutable $phpExecutable, $fileToCheck, $aspTags
3334
'-d error_reporting=E_ALL',
3435
'-n',
3536
'-l',
36-
escapeshellarg($fileToCheck),
37+
$fileToCheck,
3738
);
3839

3940
$this->showDeprecatedErrors = $deprecated;
@@ -42,6 +43,7 @@ public function __construct(PhpExecutable $phpExecutable, $fileToCheck, $aspTags
4243

4344
/**
4445
* @return bool
46+
* @throws
4547
*/
4648
public function containsError()
4749
{
@@ -86,6 +88,7 @@ public function getSyntaxError()
8688

8789
/**
8890
* @return bool
91+
* @throws RunTimeException
8992
*/
9093
public function isFail()
9194
{
@@ -94,6 +97,7 @@ public function isFail()
9497

9598
/**
9699
* @return bool
100+
* @throws RunTimeException
97101
*/
98102
public function isSuccess()
99103
{

src/Process/PhpExecutable.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public static function getPhpExecutable($phpExecutable)
7878
echo 'PHP;', PHP_VERSION_ID, ';', defined('HPHP_VERSION') ? HPHP_VERSION : null;
7979
PHP;
8080

81-
$process = new Process(escapeshellarg($phpExecutable) . ' -n -r ' . escapeshellarg($codeToExecute));
81+
$process = new Process($phpExecutable, array('-n', '-r', $codeToExecute));
8282
$process->waitForFinish();
8383

8484
try {
@@ -90,7 +90,7 @@ public static function getPhpExecutable($phpExecutable)
9090

9191
} catch (RunTimeException $e) {
9292
// Try HHVM type
93-
$process = new Process(escapeshellarg($phpExecutable) . ' --php -r ' . escapeshellarg($codeToExecute));
93+
$process = new Process($phpExecutable, array('--php', '-r', $codeToExecute));
9494
$process->waitForFinish();
9595

9696
if ($process->getStatusCode() !== 0 && $process->getStatusCode() !== 255) {

src/Process/PhpProcess.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,25 @@ class PhpProcess extends Process
88
* @param PhpExecutable $phpExecutable
99
* @param array $parameters
1010
* @param string|null $stdIn
11+
* @throws \JakubOnderka\PhpParallelLint\RunTimeException
1112
*/
1213
public function __construct(PhpExecutable $phpExecutable, array $parameters = array(), $stdIn = null)
1314
{
1415
$constructedParameters = $this->constructParameters($parameters, $phpExecutable->isIsHhvmType());
15-
$cmdLine = escapeshellcmd($phpExecutable->getPath()) . ' ' . $constructedParameters;
16-
parent::__construct($cmdLine, $stdIn);
16+
parent::__construct($phpExecutable->getPath(), $constructedParameters, $stdIn);
1717
}
1818

1919
/**
2020
* @param array $parameters
2121
* @param bool $isHhvm
22-
* @return string
22+
* @return array
2323
*/
2424
private function constructParameters(array $parameters, $isHhvm)
2525
{
26-
return ($isHhvm ? '--php ' : '') . implode(' ', $parameters);
26+
if ($isHhvm) {
27+
$parameters = array_merge(array('-php'), $parameters);
28+
}
29+
30+
return $parameters;
2731
}
28-
}
32+
}

src/Process/Process.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,20 @@ class Process
3232
private $statusCode;
3333

3434
/**
35-
* @param string $cmdLine
35+
* @param string $executable
36+
* @param string[] $arguments
3637
* @param string $stdInInput
3738
* @throws RunTimeException
3839
*/
39-
public function __construct($cmdLine, $stdInInput = null)
40+
public function __construct($executable, array $arguments = array(), $stdInInput = null)
4041
{
4142
$descriptors = array(
4243
self::STDIN => array('pipe', self::READ),
4344
self::STDOUT => array('pipe', self::WRITE),
4445
self::STDERR => array('pipe', self::WRITE),
4546
);
4647

48+
$cmdLine = $executable . ' ' . implode(' ', array_map('escapeshellarg', $arguments));
4749
$this->process = proc_open($cmdLine, $descriptors, $pipes, null, null, array('bypass_shell' => true));
4850

4951
if ($this->process === false || $this->process === null) {
@@ -142,6 +144,7 @@ public function getStatusCode()
142144

143145
/**
144146
* @return bool
147+
* @throws RunTimeException
145148
*/
146149
public function isFail()
147150
{

src/Process/SkipLintProcess.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,13 @@ public function __construct(PhpExecutable $phpExecutable, array $filesToCheck)
3030

3131
$script = str_replace('<?php', '', $script);
3232

33-
$parameters = array('-d display_errors=stderr', '-r ' . escapeshellarg($script));
34-
33+
$parameters = array('-d', 'display_errors=stderr', '-r', $script);
3534
parent::__construct($phpExecutable, $parameters, implode(PHP_EOL, $filesToCheck));
3635
}
3736

37+
/**
38+
* @throws RunTimeException
39+
*/
3840
public function getChunk()
3941
{
4042
if (!$this->isFinished()) {

0 commit comments

Comments
 (0)