CVE-2022-25866: Improper Neutralization of Argument Delimiters in a Command ('Argument Injection') (CWE-88)
value: The package czproject/git-php before 4.0.3 are vulnerable to Command Injection via git argument injection. When calling the isRemoteUrlReadable($url, array $refs = NULL) function, both the url and refs parameters are passed to the git ls-remote subcommand in a way that additional flags can be set. The additional flags can be used to perform a command injection.
CWE description: The product constructs a string for a command to be executed by a separate component
in another control sphere, but it does not properly delimit the
intended arguments, options, or switches within that command string.
CVSS v3 base score: 9.8
Published: 2022-04-25T17:15Z
Repository: https://github.com/czproject/git-php
Language: Other
Files changed: src/Git.php, src/GitRepository.php, tests/GitPhp/GitRepository.branches.phpt, tests/GitPhp/GitRepository.files.phpt, tests/GitPhp/GitRepository.remotes.phpt, tests/GitPhp/GitRepository.tags.phpt
Commit date: 2022-04-21 18:50:57 +0200
Commit message: Uses --end-of-options after command options (for security reasons)

Diff: diff --git a/src/Git.php b/src/Git.php
index 89f9b1f..575c7c9 100644
--- a/src/Git.php
+++ b/src/Git.php
@@ -44,10 +44,11 @@
 
 			try {
 				$this->run($directory, [
 					'init',
 					$params,
+					'--end-of-options',
 					$directory
 				]);
 
 			} catch (GitException $e) {
 				throw new GitException("Git init failed (directory $directory).", $e->getCode(), $e);
@@ -87,10 +88,11 @@
 
 			try {
 				$this->run($cwd, [
 					'clone',
 					$params,
+					'--end-of-options',
 					$url,
 					$directory
 				]);
 
 			} catch (GitException $e) {
@@ -118,10 +120,11 @@
 			$result = $this->runner->run($this->runner->getCwd(), [
 				'ls-remote',
 				'--heads',
 				'--quiet',
 				'--exit-code',
+				'--end-of-options',
 				$url,
 				$refs,
 			], [
 				'GIT_TERMINAL_PROMPT' => 0,
 			]);
diff --git a/src/GitRepository.php b/src/GitRepository.php
index f8ec97b..e677f3e 100644
--- a/src/GitRepository.php
+++ b/src/GitRepository.php
@@ -50,11 +50,11 @@
 		 * @throws GitException
 		 * @return static
 		 */
 		public function createTag($name, $options = NULL)
 		{
-			$this->run('tag', $options, $name);
+			$this->run('tag', $options, '--end-of-options', $name);
 			return $this;
 		}
 
 
 		/**
@@ -84,11 +84,11 @@
 		 */
 		public function renameTag($oldName, $newName)
 		{
 			// http://stackoverflow.com/a/1873932
 			// create new as alias to old (`git tag NEW OLD`)
-			$this->run('tag', $newName, $oldName);
+			$this->run('tag', '--end-of-options', $newName, $oldName);
 			// delete old (`git tag -d OLD`)
 			$this->removeTag($oldName);
 			return $this;
 		}
 
@@ -112,11 +112,11 @@
 		 * @throws GitException
 		 * @return static
 		 */
 		public function merge($branch, $options = NULL)
 		{
-			$this->run('merge', $options, $branch);
+			$this->run('merge', $options, '--end-of-options', $branch);
 			return $this;
 		}
 
 
 		/**
@@ -129,11 +129,11 @@
 		 * @return static
 		 */
 		public function createBranch($name, $checkout = FALSE)
 		{
 			// git branch $name
-			$this->run('branch', $name);
+			$this->run('branch', '--end-of-options', $name);
 
 			if ($checkout) {
 				$this->checkout($name);
 			}
 
@@ -232,11 +232,11 @@
 		 * @throws GitException
 		 * @return static
 		 */
 		public function checkout($name)
 		{
-			$this->run('checkout', $name);
+			$this->run('checkout', '--end-of-options', $name);
 			return $this;
 		}
 
 
 		/**
@@ -251,11 +251,11 @@
 			if (!is_array($file)) {
 				$file = func_get_args();
 			}
 
 			foreach ($file as $item) {
-				$this->run('rm', $item, '-r');
+				$this->run('rm', '-r', '--end-of-options', $item);
 			}
 
 			return $this;
 		}
 
@@ -280,11 +280,11 @@
 
 				if (!file_exists($path)) {
 					throw new GitException("The path at '$item' does not represent a valid file.");
 				}
 
-				$this->run('add', $item);
+				$this->run('add', '--end-of-options', $item);
 			}
 
 			return $this;
 		}
 
@@ -317,11 +317,11 @@
 					$file => $to,
 				];
 			}
 
 			foreach ($file as $from => $to) {
-				$this->run('mv', $from, $to);
+				$this->run('mv', '--end-of-options', $from, $to);
 			}
 
 			return $this;
 		}
 
@@ -452,11 +452,11 @@
 		 * @return static
 		 * @throws GitException
 		 */
 		public function pull($remote = NULL, array $params = NULL)
 		{
-			$this->run('pull', $remote, $params);
+			$this->run('pull', $params, '--end-of-options', $remote);
 			return $this;
 		}
 
 
 		/**
@@ -466,11 +466,11 @@
 		 * @return static
 		 * @throws GitException
 		 */
 		public function push($remote = NULL, array $params = NULL)
 		{
-			$this->run('push', $remote, $params);
+			$this->run('push', $params, '--end-of-options', $remote);
 			return $this;
 		}
 
 
 		/**
@@ -480,11 +480,11 @@
 		 * @return static
 		 * @throws GitException
 		 */
 		public function fetch($remote = NULL, array $params = NULL)
 		{
-			$this->run('fetch', $remote, $params);
+			$this->run('fetch', $params, '--end-of-options', $remote);
 			return $this;
 		}
 
 
 		/**
@@ -495,11 +495,11 @@
 		 * @return static
 		 * @throws GitException
 		 */
 		public function addRemote($name, $url, array $params = NULL)
 		{
-			$this->run('remote', 'add', $params, $name, $url);
+			$this->run('remote', 'add', $params, '--end-of-options', $name, $url);
 			return $this;
 		}
 
 
 		/**
@@ -509,11 +509,11 @@
 		 * @return static
 		 * @throws GitException
 		 */
 		public function renameRemote($oldName, $newName)
 		{
-			$this->run('remote', 'rename', $oldName, $newName);
+			$this->run('remote', 'rename', '--end-of-options', $oldName, $newName);
 			return $this;
 		}
 
 
 		/**
@@ -522,11 +522,11 @@
 		 * @return static
 		 * @throws GitException
 		 */
 		public function removeRemote($name)
 		{
-			$this->run('remote', 'remove', $name);
+			$this->run('remote', 'remove', '--end-of-options', $name);
 			return $this;
 		}
 
 
 		/**
@@ -537,11 +537,11 @@
 		 * @return static
 		 * @throws GitException
 		 */
 		public function setRemoteUrl($name, $url, array $params = NULL)
 		{
-			$this->run('remote', 'set-url', $params, $name, $url);
+			$this->run('remote', 'set-url', $params, '--end-of-options', $name, $url);
 			return $this;
 		}
 
 
 		/**
diff --git a/tests/GitPhp/GitRepository.branches.phpt b/tests/GitPhp/GitRepository.branches.phpt
index 8088e6b..3b41d48 100644
--- a/tests/GitPhp/GitRepository.branches.phpt
+++ b/tests/GitPhp/GitRepository.branches.phpt
@@ -8,16 +8,16 @@ use CzProject\GitPhp\Tests\AssertRunner;
 require __DIR__ . '/bootstrap.php';
 
 $runner = new AssertRunner(__DIR__);
 $git = new Git($runner);
 
-$runner->assert(['branch', 'master']);
-$runner->assert(['branch', 'develop']);
-$runner->assert(['checkout', 'develop']);
-$runner->assert(['merge', 'feature-1']);
+$runner->assert(['branch', '--end-of-options', 'master']);
+$runner->assert(['branch', '--end-of-options', 'develop']);
+$runner->assert(['checkout', '--end-of-options', 'develop']);
+$runner->assert(['merge', '--end-of-options', 'feature-1']);
 $runner->assert(['branch', '-d', 'feature-1']);
-$runner->assert(['checkout', 'master']);
+$runner->assert(['checkout', '--end-of-options', 'master']);
 
 $repo = $git->open(__DIR__);
 $repo->createBranch('master');
 $repo->createBranch('develop', TRUE);
 $repo->merge('feature-1');
diff --git a/tests/GitPhp/GitRepository.files.phpt b/tests/GitPhp/GitRepository.files.phpt
index 5e1deac..5ab28ea 100644
--- a/tests/GitPhp/GitRepository.files.phpt
+++ b/tests/GitPhp/GitRepository.files.phpt
@@ -12,15 +12,15 @@ $git = new Git($runner);
 $repo = $git->open(__DIR__ . '/fixtures');
 
 
 test(function () use ($repo, $runner) {
 	$runner->resetAsserts();
-	$runner->assert(['add', 'file1.txt']);
-	$runner->assert(['add', 'file2.txt']);
-	$runner->assert(['add', 'file3.txt']);
-	$runner->assert(['add', 'file4.txt']);
-	$runner->assert(['add', 'file5.txt']);
+	$runner->assert(['add', '--end-of-options', 'file1.txt']);
+	$runner->assert(['add', '--end-of-options', 'file2.txt']);
+	$runner->assert(['add', '--end-of-options', 'file3.txt']);
+	$runner->assert(['add', '--end-of-options', 'file4.txt']);
+	$runner->assert(['add', '--end-of-options', 'file5.txt']);
 
 	$repo->addFile('file1.txt');
 	$repo->addFile([
 		'file2.txt',
 		'file3.txt',
@@ -36,15 +36,15 @@ test(function () use ($repo) {
 });
 
 
 test(function () use ($repo, $runner) {
 	$runner->resetAsserts();
-	$runner->assert(['rm', 'file1.txt', '-r']);
-	$runner->assert(['rm', 'file2.txt', '-r']);
-	$runner->assert(['rm', 'file3.txt', '-r']);
-	$runner->assert(['rm', 'file4.txt', '-r']);
-	$runner->assert(['rm', 'file5.txt', '-r']);
+	$runner->assert(['rm', '-r', '--end-of-options', 'file1.txt']);
+	$runner->assert(['rm', '-r', '--end-of-options', 'file2.txt']);
+	$runner->assert(['rm', '-r', '--end-of-options', 'file3.txt']);
+	$runner->assert(['rm', '-r', '--end-of-options', 'file4.txt']);
+	$runner->assert(['rm', '-r', '--end-of-options', 'file5.txt']);
 
 	$repo->removeFile('file1.txt');
 	$repo->removeFile([
 		'file2.txt',
 		'file3.txt',
@@ -53,13 +53,13 @@ test(function () use ($repo, $runner) {
 });
 
 
 test(function () use ($repo, $runner) {
 	$runner->resetAsserts();
-	$runner->assert(['mv', 'file1.txt', 'new1.txt']);
-	$runner->assert(['mv', 'file2.txt', 'new2.txt']);
-	$runner->assert(['mv', 'file3.txt', 'new3.txt']);
+	$runner->assert(['mv', '--end-of-options', 'file1.txt', 'new1.txt']);
+	$runner->assert(['mv', '--end-of-options', 'file2.txt', 'new2.txt']);
+	$runner->assert(['mv', '--end-of-options', 'file3.txt', 'new3.txt']);
 
 	$repo->renameFile('file1.txt', 'new1.txt');
 	$repo->renameFile([
 		'file2.txt' => 'new2.txt',
 		'file3.txt' => 'new3.txt',
diff --git a/tests/GitPhp/GitRepository.remotes.phpt b/tests/GitPhp/GitRepository.remotes.phpt
index 1f090ff..d85fd58 100644
--- a/tests/GitPhp/GitRepository.remotes.phpt
+++ b/tests/GitPhp/GitRepository.remotes.phpt
@@ -8,21 +8,21 @@ use CzProject\GitPhp\Tests\AssertRunner;
 require __DIR__ . '/bootstrap.php';
 
 $runner = new AssertRunner(__DIR__);
 $git = new Git($runner);
 
-$runner->assert(['clone', '-q', 'git@github.com:czproject/git-php.git', __DIR__]);
-$runner->assert(['remote', 'add', 'origin2', 'git@github.com:czproject/git-php.git']);
-$runner->assert(['remote', 'add', 'remote', 'git@github.com:czproject/git-php.git']);
+$runner->assert(['clone', '-q', '--end-of-options', 'git@github.com:czproject/git-php.git', __DIR__]);
+$runner->assert(['remote', 'add', '--end-of-options', 'origin2', 'git@github.com:czproject/git-php.git']);
+$runner->assert(['remote', 'add', '--end-of-options', 'remote', 'git@github.com:czproject/git-php.git']);
 $runner->assert(['remote', 'add', [
 	'--mirror=push',
-], 'only-push', 'test-url']);
-$runner->assert(['remote', 'rename', 'remote', 'origin3']);
+], '--end-of-options', 'only-push', 'test-url']);
+$runner->assert(['remote', 'rename', '--end-of-options', 'remote', 'origin3']);
 $runner->assert(['remote', 'set-url', [
 	'--push',
-], 'origin3', 'test-url']);
-$runner->assert(['remote', 'remove', 'origin2']);
+], '--end-of-options', 'origin3', 'test-url']);
+$runner->assert(['remote', 'remove', '--end-of-options', 'origin2']);
 
 $repo = $git->cloneRepository('git@github.com:czproject/git-php.git', __DIR__);
 $repo->addRemote('origin2', 'git@github.com:czproject/git-php.git');
 $repo->addRemote('remote', 'git@github.com:czproject/git-php.git');
 $repo->addRemote('only-push', 'test-url', [
@@ -32,11 +32,11 @@ $repo->renameRemote('remote', 'origin3');
 $repo->setRemoteUrl('origin3', 'test-url', [
 	'--push',
 ]);
 $repo->removeRemote('origin2');
 
-$runner->assert(['push', 'origin']);
-$runner->assert(['fetch', 'origin']);
-$runner->assert(['pull', 'origin']);
+$runner->assert(['push', '--end-of-options', 'origin']);
+$runner->assert(['fetch', '--end-of-options', 'origin']);
+$runner->assert(['pull', '--end-of-options', 'origin']);
 $repo->push('origin');
 $repo->fetch('origin');
 $repo->pull('origin');
diff --git a/tests/GitPhp/GitRepository.tags.phpt b/tests/GitPhp/GitRepository.tags.phpt
index c0eb44a..508458a 100644
--- a/tests/GitPhp/GitRepository.tags.phpt
+++ b/tests/GitPhp/GitRepository.tags.phpt
@@ -8,12 +8,12 @@ use CzProject\GitPhp\Tests\AssertRunner;
 require __DIR__ . '/bootstrap.php';
 
 $runner = new AssertRunner(__DIR__);
 $git = new Git($runner);
 
-$runner->assert(['tag', 'v1.0.0']);
-$runner->assert(['tag', 'v2.0.0', 'v1.0.0']);
+$runner->assert(['tag', '--end-of-options', 'v1.0.0']);
+$runner->assert(['tag', '--end-of-options', 'v2.0.0', 'v1.0.0']);
 $runner->assert(['tag', '-d', 'v1.0.0']);
 $runner->assert(['tag', '-d', 'v2.0.0']);
 
 $repo = $git->open(__DIR__);
 $repo->createTag('v1.0.0');

Vulnerable code: 			$this->run('tag', $options, $name);
			$this->run('tag', $newName, $oldName);
			$this->run('merge', $options, $branch);
			$this->run('branch', $name);
			$this->run('checkout', $name);
				$this->run('rm', $item, '-r');
				$this->run('add', $item);
				$this->run('mv', $from, $to);
			$this->run('pull', $remote, $params);
			$this->run('push', $remote, $params);
			$this->run('fetch', $remote, $params);
			$this->run('remote', 'add', $params, $name, $url);
			$this->run('remote', 'rename', $oldName, $newName);
			$this->run('remote', 'remove', $name);
			$this->run('remote', 'set-url', $params, $name, $url);
$runner->assert(['branch', 'master']);
$runner->assert(['branch', 'develop']);
$runner->assert(['checkout', 'develop']);
$runner->assert(['merge', 'feature-1']);
$runner->assert(['checkout', 'master']);
	$runner->assert(['add', 'file1.txt']);
	$runner->assert(['add', 'file2.txt']);
	$runner->assert(['add', 'file3.txt']);
	$runner->assert(['add', 'file4.txt']);
	$runner->assert(['add', 'file5.txt']);
	$runner->assert(['rm', 'file1.txt', '-r']);
	$runner->assert(['rm', 'file2.txt', '-r']);
	$runner->assert(['rm', 'file3.txt', '-r']);
	$runner->assert(['rm', 'file4.txt', '-r']);
	$runner->assert(['rm', 'file5.txt', '-r']);
	$runner->assert(['mv', 'file1.txt', 'new1.txt']);
	$runner->assert(['mv', 'file2.txt', 'new2.txt']);
	$runner->assert(['mv', 'file3.txt', 'new3.txt']);
$runner->assert(['clone', '-q', 'git@github.com:czproject/git-php.git', __DIR__]);
$runner->assert(['remote', 'add', 'origin2', 'git@github.com:czproject/git-php.git']);
$runner->assert(['remote', 'add', 'remote', 'git@github.com:czproject/git-php.git']);
], 'only-push', 'test-url']);
$runner->assert(['remote', 'rename', 'remote', 'origin3']);
], 'origin3', 'test-url']);
$runner->assert(['remote', 'remove', 'origin2']);
$runner->assert(['push', 'origin']);
$runner->assert(['fetch', 'origin']);
$runner->assert(['pull', 'origin']);
$runner->assert(['tag', 'v1.0.0']);
$runner->assert(['tag', 'v2.0.0', 'v1.0.0']);
Fixed code: 					'--end-of-options',
					'--end-of-options',
				'--end-of-options',
			$this->run('tag', $options, '--end-of-options', $name);
			$this->run('tag', '--end-of-options', $newName, $oldName);
			$this->run('merge', $options, '--end-of-options', $branch);
			$this->run('branch', '--end-of-options', $name);
			$this->run('checkout', '--end-of-options', $name);
				$this->run('rm', '-r', '--end-of-options', $item);
				$this->run('add', '--end-of-options', $item);
				$this->run('mv', '--end-of-options', $from, $to);
			$this->run('pull', $params, '--end-of-options', $remote);
			$this->run('push', $params, '--end-of-options', $remote);
			$this->run('fetch', $params, '--end-of-options', $remote);
			$this->run('remote', 'add', $params, '--end-of-options', $name, $url);
			$this->run('remote', 'rename', '--end-of-options', $oldName, $newName);
			$this->run('remote', 'remove', '--end-of-options', $name);
			$this->run('remote', 'set-url', $params, '--end-of-options', $name, $url);
$runner->assert(['branch', '--end-of-options', 'master']);
$runner->assert(['branch', '--end-of-options', 'develop']);
$runner->assert(['checkout', '--end-of-options', 'develop']);
$runner->assert(['merge', '--end-of-options', 'feature-1']);
$runner->assert(['checkout', '--end-of-options', 'master']);
	$runner->assert(['add', '--end-of-options', 'file1.txt']);
	$runner->assert(['add', '--end-of-options', 'file2.txt']);
	$runner->assert(['add', '--end-of-options', 'file3.txt']);
	$runner->assert(['add', '--end-of-options', 'file4.txt']);
	$runner->assert(['add', '--end-of-options', 'file5.txt']);
	$runner->assert(['rm', '-r', '--end-of-options', 'file1.txt']);
	$runner->assert(['rm', '-r', '--end-of-options', 'file2.txt']);
	$runner->assert(['rm', '-r', '--end-of-options', 'file3.txt']);
	$runner->assert(['rm', '-r', '--end-of-options', 'file4.txt']);
	$runner->assert(['rm', '-r', '--end-of-options', 'file5.txt']);
	$runner->assert(['mv', '--end-of-options', 'file1.txt', 'new1.txt']);
	$runner->assert(['mv', '--end-of-options', 'file2.txt', 'new2.txt']);
	$runner->assert(['mv', '--end-of-options', 'file3.txt', 'new3.txt']);
$runner->assert(['clone', '-q', '--end-of-options', 'git@github.com:czproject/git-php.git', __DIR__]);
$runner->assert(['remote', 'add', '--end-of-options', 'origin2', 'git@github.com:czproject/git-php.git']);
$runner->assert(['remote', 'add', '--end-of-options', 'remote', 'git@github.com:czproject/git-php.git']);
], '--end-of-options', 'only-push', 'test-url']);
$runner->assert(['remote', 'rename', '--end-of-options', 'remote', 'origin3']);
], '--end-of-options', 'origin3', 'test-url']);
$runner->assert(['remote', 'remove', '--end-of-options', 'origin2']);
$runner->assert(['push', '--end-of-options', 'origin']);
$runner->assert(['fetch', '--end-of-options', 'origin']);
$runner->assert(['pull', '--end-of-options', 'origin']);
$runner->assert(['tag', '--end-of-options', 'v1.0.0']);
$runner->assert(['tag', '--end-of-options', 'v2.0.0', 'v1.0.0']);