diff --git a/src/Command.php b/src/Command.php index 7b78fbb..ece7c9a 100644 --- a/src/Command.php +++ b/src/Command.php @@ -272,8 +272,8 @@ public function getArgs() * @param string|array|null $value the optional argument value which will * get escaped if $escapeArgs is true. An array can be passed to add more * than one value for a key, e.g. `addArg('--exclude', - * array('val1','val2'))` which will create the option `--exclude 'val1' - * 'val2'`. + * array('val1','val2'))` which will create the option `'--exclude' 'val1' + * '--exclude' 'val2'`. * @param bool|null $escape if set, this overrides the $escapeArgs setting * and enforces escaping/no escaping * @return static for method chaining @@ -288,19 +288,22 @@ public function addArg($key, $value = null, $escape = null) setlocale(LC_CTYPE, $this->locale); } if ($value === null) { - // Only escape single arguments if explicitely requested - $this->_args[] = $escape ? escapeshellarg($key) : $key; + $this->_args[] = $doEscape ? escapeshellarg($key) : $key; } else { - $separator = substr($key, -1)==='=' ? '' : ' '; if (is_array($value)) { - $params = array(); foreach ($value as $v) { - $params[] = $doEscape ? escapeshellarg($v) : $v; + $this->addArg($key, $v, $escape); } - $this->_args[] = $key . $separator.implode(' ',$params); } else { - $this->_args[] = $key . $separator . - ($doEscape ? escapeshellarg($value) : $value); + if (substr($key, -1) === '=') { + $this->_args[] = $doEscape ? + escapeshellarg($key . $value) : + $key . $value; + } else { + $this->_args[] = $doEscape ? + escapeshellarg($key) . ' ' . escapeshellarg($value) : + $key . ' ' . $value; + } } } if ($useLocale) { diff --git a/tests/BlockingCommandTest.php b/tests/BlockingCommandTest.php index 25ac7a2..838c98c 100644 --- a/tests/BlockingCommandTest.php +++ b/tests/BlockingCommandTest.php @@ -62,7 +62,7 @@ public function testCanRunCommandWithArguments() $command->nonBlockingMode = false; $command->addArg('-l'); $command->addArg('-n'); - $this->assertEquals("ls -l -n", $command->getExecCommand()); + $this->assertEquals("ls '-l' '-n'", $command->getExecCommand()); $this->assertFalse($command->getExecuted()); $this->assertTrue($command->execute()); $this->assertTrue($command->getExecuted()); diff --git a/tests/CommandTest.php b/tests/CommandTest.php index 2797fcb..a141883 100644 --- a/tests/CommandTest.php +++ b/tests/CommandTest.php @@ -81,8 +81,8 @@ public function testCanAddArguments() $command->addArg('-b=', array('v4','v5','v6')); $command->addArg('-c', ''); $command->addArg('some name', null, true); - $this->assertEquals("--arg1=x --a --a '中文字äüp' --a 'v'\''1' 'v2' 'v3' -b=v -b='v4' 'v5' 'v6' -c '' 'some name'", $command->getArgs()); - $this->assertEquals("test --arg1=x --a --a '中文字äüp' --a 'v'\''1' 'v2' 'v3' -b=v -b='v4' 'v5' 'v6' -c '' 'some name'", $command->getExecCommand()); + $this->assertEquals("--arg1=x '--a' '--a' '中文字äüp' '--a' 'v'\''1' '--a' 'v2' '--a' 'v3' -b=v '-b=v4' '-b=v5' '-b=v6' '-c' '' 'some name'", $command->getArgs()); + $this->assertEquals("test --arg1=x '--a' '--a' '中文字äüp' '--a' 'v'\''1' '--a' 'v2' '--a' 'v3' -b=v '-b=v4' '-b=v5' '-b=v6' '-c' '' 'some name'", $command->getExecCommand()); } public function testCanResetArguments() { @@ -102,14 +102,29 @@ public function testCanDisableEscaping() $command->addArg('-b=','v', true); $command->addArg('-b=', array('v4','v5','v6')); $command->addArg('some name', null, true); - $this->assertEquals("--a --a v --a v1 v2 v3 -b='v' -b=v4 v5 v6 'some name'", $command->getArgs()); + $this->assertEquals("--a --a v --a v1 --a v2 --a v3 '-b=v' -b=v4 -b=v5 -b=v6 'some name'", $command->getArgs()); + } + public function testCanPreventCommandInjection() + { + $command = new Command(array( + 'command' => 'curl', + )); + $command->addArg('http://example.com --wrong-argument || echo "RCE 1"'); + $this->assertEquals("'http://example.com --wrong-argument || echo \"RCE 1\"'", $command->getArgs()); + + $command = new Command(array( + 'command' => 'curl', + )); + $command->addArg('http://example.com'); + $command->addArg('--header foo --wrong-argument || echo "RCE 2" ||', 'bar'); + $this->assertEquals("'http://example.com' '--header foo --wrong-argument || echo \"RCE 2\" ||' 'bar'", $command->getArgs()); } public function testCanRunCommandWithArguments() { $command = new Command('ls'); $command->addArg('-l'); $command->addArg('-n'); - $this->assertEquals("ls -l -n", $command->getExecCommand()); + $this->assertEquals("ls '-l' '-n'", $command->getExecCommand()); $this->assertFalse($command->getExecuted()); $this->assertTrue($command->execute()); $this->assertTrue($command->getExecuted()); @@ -163,7 +178,7 @@ public function testCanCastToString() $command = new Command('ls'); $command->addArg('-l'); $command->addArg('-n'); - $this->assertEquals("ls -l -n", (string)$command); + $this->assertEquals("ls '-l' '-n'", (string)$command); } // Exec