CVE-2017-1000214: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: GitPHP by xiphux is vulnerable to OS Command Injections
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2017-11-27T14:29Z
Repository: https://github.com/Enalean/gitphp
Language: PHP
Files changed: include/controller/Controller_GraphData.class.php, include/git/FileBlame.class.php, include/git/FileHistory.class.php, include/git/FileSearch.class.php, include/git/TreeDiff.class.php, include/git/blob/BlobLoad_Base.class.php, include/git/commit/CommitLoad_Base.class.php, include/git/project/ProjectLoad_Git.class.php, include/git/reflist/RefListLoad_Git.class.php, include/git/revlist/RevList_Git.class.php, include/git/tag/TagLoad_Git.class.php, include/git/tree/TreeLoad_Base.class.php, include/git/tree/TreeLoad_Git.class.php
Commit date: 2017-07-07 14:11:47 +0200
Commit message: Fix shell injections

No dynamic parameters should be passed shell_exec() without
being properly escaped

Diff: diff --git a/include/controller/Controller_GraphData.class.php b/include/controller/Controller_GraphData.class.php
index f078819..70df10a 100644
--- a/include/controller/Controller_GraphData.class.php
+++ b/include/controller/Controller_GraphData.class.php
@@ -74,11 +74,11 @@ class GitPHP_Controller_GraphData extends GitPHP_ControllerBase
 
 		if ($this->params['graphtype'] == 'commitactivity') {
 
 			$data = array();
 
-			$commits = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'rev-list', array('--format=format:"%H %ct"', $head->GetHash())));
+			$commits = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'rev-list', array('--format=format:"%H %ct"', escapeshellarg($head->GetHash()))));
 			foreach ($commits as $commit) {
 				if (preg_match('/^([0-9a-fA-F]{40}) ([0-9]+)$/', $commit, $regs)) {
 					$data[] = array('CommitEpoch' => (int)$regs[2]);
 				}
 			}
@@ -88,11 +88,11 @@ class GitPHP_Controller_GraphData extends GitPHP_ControllerBase
 			$data = array();
 
 			include_once(GITPHP_GESHIDIR . "geshi.php");
 			$geshi = new GeSHi("",'php');
 
-			$files = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'ls-tree', array('-r', '--name-only', $head->GetTree()->GetHash())));
+			$files = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'ls-tree', array('-r', '--name-only', escapeshellarg($head->GetTree()->GetHash()))));
 			foreach ($files as $file) {
 				$filename = GitPHP_Util::BaseName($file);
 				$lang = GitPHP_Util::GeshiFilenameToLanguage($filename);
 				if (empty($lang)) {
 					$lang = $geshi->get_language_name_from_extension(substr(strrchr($filename, '.'), 1));
diff --git a/include/git/FileBlame.class.php b/include/git/FileBlame.class.php
index 2040426..2ba440f 100644
--- a/include/git/FileBlame.class.php
+++ b/include/git/FileBlame.class.php
@@ -139,13 +139,13 @@ class GitPHP_FileBlame
 		$this->dataLoaded = true;
 
 		$args = array();
 		$args[] = '-s';
 		$args[] = '-l';
-		$args[] = $this->commitHash;
+		$args[] = escapeshellarg($this->commitHash);
 		$args[] = '--';
-		$args[] = $this->path;
+		$args[] = escapeshellarg($this->path);
 
 		$blamelines = explode("\n", $this->exe->Execute($this->project->GetPath(), GIT_BLAME, $args));
 
 		$lastcommit = '';
 		foreach ($blamelines as $line) {
diff --git a/include/git/FileHistory.class.php b/include/git/FileHistory.class.php
index 11de28e..f3081ef 100644
--- a/include/git/FileHistory.class.php
+++ b/include/git/FileHistory.class.php
@@ -263,31 +263,31 @@ class GitPHP_FileHistory implements Iterator, GitPHP_Pagination_Interface
 		if ($this->skip > 0)
 			$canSkip = $this->exe->CanSkip();
 
 		if ($canSkip) {
 			if ($this->limit > 0) {
-				$args[] = '--max-count=' . $this->limit;
+				$args[] = '--max-count=' . escapeshellarg($this->limit);
 			}
 			if ($this->skip > 0) {
-				$args[] = '--skip=' . $this->skip;
+				$args[] = '--skip=' . escapeshellarg($this->skip);
 			}
 		} else {
 			if ($this->limit > 0) {
-				$args[] = '--max-count=' . ($this->limit + $this->skip);
+				$args[] = '--max-count=' . escapeshellarg($this->limit + $this->skip);
 			}
 		}
 
 		$args[] = '--';
-		$args[] = $this->path;
+		$args[] = escapeshellarg($this->path);
 		$args[] = '|';
 		$args[] = $this->exe->GetBinary();
 		$args[] = '--git-dir=' . escapeshellarg($this->project->GetPath());
 		$args[] = GIT_DIFF_TREE;
 		$args[] = '-r';
 		$args[] = '--stdin';
 		$args[] = '--';
-		$args[] = $this->path;
+		$args[] = escapeshellarg($this->path);
 		
 		$historylines = explode("\n", $this->exe->Execute($this->project->GetPath(), GIT_REV_LIST, $args));
 
 		$commitHash = null;
 		foreach ($historylines as $line) {
diff --git a/include/git/FileSearch.class.php b/include/git/FileSearch.class.php
index f1f30d9..8e373c1 100644
--- a/include/git/FileSearch.class.php
+++ b/include/git/FileSearch.class.php
@@ -380,12 +380,12 @@ class GitPHP_FileSearch implements Iterator, GitPHP_Pagination_Interface
 		$args[] = '-I';
 		$args[] = '--full-name';
 		$args[] = '--ignore-case';
 		$args[] = '-n';
 		$args[] = '-e';
-		$args[] = '"' . addslashes($this->search) . '"';
-		$args[] = $this->treeHash;
+		$args[] = escapeshellarg($this->search);
+		$args[] = escapeshellarg($this->treeHash);
 
 		$lines = explode("\n", $this->exe->Execute($this->project->GetPath(), GIT_GREP, $args));
 
 
 		foreach ($lines as $line) {
diff --git a/include/git/TreeDiff.class.php b/include/git/TreeDiff.class.php
index bd3e8cf..573f0d3 100644
--- a/include/git/TreeDiff.class.php
+++ b/include/git/TreeDiff.class.php
@@ -119,13 +119,13 @@ class GitPHP_TreeDiff implements Iterator
 			$args[] = '-M';
 
 		if (empty($this->fromHash))
 			$args[] = '--root';
 		else
-			$args[] = $this->fromHash;
+			$args[] = escapeshellarg($this->fromHash);
 
-		$args[] = $this->toHash;
+		$args[] = escapeshellarg($this->toHash);
 
 		$diffTreeLines = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), GIT_DIFF_TREE, $args));
 		foreach ($diffTreeLines as $line) {
 			$trimmed = trim($line);
 			if ((strlen($trimmed) > 0) && (substr_compare($trimmed, ':', 0, 1) === 0)) {
diff --git a/include/git/blob/BlobLoad_Base.class.php b/include/git/blob/BlobLoad_Base.class.php
index eb0bb39..424073a 100644
--- a/include/git/blob/BlobLoad_Base.class.php
+++ b/include/git/blob/BlobLoad_Base.class.php
@@ -40,10 +40,10 @@ abstract class GitPHP_BlobLoad_Base implements GitPHP_BlobLoadStrategy_Interface
 		if (!$blob)
 			return;
 
 		$args = array();
 		$args[] = '-s';
-		$args[] = $blob->GetHash();
+		$args[] = escapeshellarg($blob->GetHash());
 
 		return $this->exe->Execute($blob->GetProject()->GetPath(), GIT_CAT_FILE, $args);
 	}
 }
diff --git a/include/git/commit/CommitLoad_Base.class.php b/include/git/commit/CommitLoad_Base.class.php
index df6b265..c7d0e91 100644
--- a/include/git/commit/CommitLoad_Base.class.php
+++ b/include/git/commit/CommitLoad_Base.class.php
@@ -40,11 +40,11 @@ abstract class GitPHP_CommitLoad_Base implements GitPHP_CommitLoadStrategy_Inter
 		if (!$commit)
 			return;
 
 		$args = array();
 		$args[] = '--tags';
-		$args[] = $commit->GetHash();
+		$args[] = escapeshellarg($commit->GetHash());
 		$revs = explode("\n", $this->exe->Execute($commit->GetProject()->GetPath(), GIT_NAME_REV, $args));
 
 		foreach ($revs as $revline) {
 			if (preg_match('/^([0-9a-fA-F]{40})\s+tags\/(.+)(\^[0-9]+|\~[0-9]+)$/', $revline, $regs)) {
 				if ($regs[1] == $commit->GetHash()) {
diff --git a/include/git/project/ProjectLoad_Git.class.php b/include/git/project/ProjectLoad_Git.class.php
index 70f642a..b64b7e1 100644
--- a/include/git/project/ProjectLoad_Git.class.php
+++ b/include/git/project/ProjectLoad_Git.class.php
@@ -87,11 +87,11 @@ class GitPHP_ProjectLoad_Git implements GitPHP_ProjectLoadStrategy_Interface
 		}
 
 		$args = array();
 		$args[] = '-1';
 		$args[] = '--format=format:%H';
-		$args[] = $abbrevHash;
+		$args[] = escapeshellarg($abbrevHash);
 
 		$fullData = explode("\n", $this->exe->Execute($project->GetPath(), GIT_REV_LIST, $args));
 		if (empty($fullData[0])) {
 			return $abbrevHash;
 		}
@@ -123,11 +123,11 @@ class GitPHP_ProjectLoad_Git implements GitPHP_ProjectLoadStrategy_Interface
 		}
 
 		$args = array();
 		$args[] = '-1';
 		$args[] = '--format=format:%h';
-		$args[] = $hash;
+		$args[] = escapeshellarg($hash);
 
 		$abbrevData = explode("\n", $this->exe->Execute($project->GetPath(), GIT_REV_LIST, $args));
 		if (empty($abbrevData[0])) {
 			return $hash;
 		}
diff --git a/include/git/reflist/RefListLoad_Git.class.php b/include/git/reflist/RefListLoad_Git.class.php
index 805d3fc..b3c61c4 100644
--- a/include/git/reflist/RefListLoad_Git.class.php
+++ b/include/git/reflist/RefListLoad_Git.class.php
@@ -43,11 +43,11 @@ abstract class GitPHP_RefListLoad_Git
 
 		if (empty($type))
 			return;
 
 		$args = array();
-		$args[] = '--' . $type;
+		$args[] = '--' . escapeshellarg($type);
 		$args[] = '--dereference';
 		$ret = $this->exe->Execute($refList->GetProject()->GetPath(), GIT_SHOW_REF, $args);
 
 		$lines = explode("\n", $ret);
 
@@ -84,21 +84,21 @@ abstract class GitPHP_RefListLoad_Git
 
 		if (empty($type) || empty($order))
 			return null;
 
 		$args = array();
-		$args[] = '--sort=' . $order;
+		$args[] = '--sort=' . escapeshellarg($order);
 		$args[] = '--format="%(refname)"';
 		if ($count > 0) {
 			if ($skip > 0) {
-				$args[] = '--count=' . ($count + $skip);
+				$args[] = '--count=' . escapeshellarg($count + $skip);
 			} else {
-				$args[] = '--count=' . $count;
+				$args[] = '--count=' . escapeshellarg($count);
 			}
 		}
 		$args[] = '--';
-		$args[] = 'refs/' . $type;
+		$args[] = escapeshellarg('refs/' . $type);
 		$ret = $this->exe->Execute($refList->GetProject()->GetPath(), GIT_FOR_EACH_REF, $args);
 
 		$lines = explode("\n", $ret);
 
 		$prefix = 'refs/' . $type . '/';
diff --git a/include/git/revlist/RevList_Git.class.php b/include/git/revlist/RevList_Git.class.php
index 5593ed2..5684601 100644
--- a/include/git/revlist/RevList_Git.class.php
+++ b/include/git/revlist/RevList_Git.class.php
@@ -50,18 +50,18 @@ class GitPHP_RevList_Git
 
 		$extraargs = array();
 
 		if ($canSkip) {
 			if ($count > 0) {
-				$extraargs[] = '--max-count=' . $count;
+				$extraargs[] = '--max-count=' . escapeshellarg($count);
 			}
 			if ($skip > 0) {
-				$extraargs[] = '--skip=' . $skip;
+				$extraargs[] = '--skip=' . escapeshellarg($skip);
 			}
 		} else {
 			if ($count > 0) {
-				$extraargs[] = '--max-count=' . ($count + $skip);
+				$extraargs[] = '--max-count=' . escapeshellarg($count + $skip);
 			}
 		}
 
 		$extraargs[] = $hash;
 
diff --git a/include/git/tag/TagLoad_Git.class.php b/include/git/tag/TagLoad_Git.class.php
index e569751..3ac6eee 100644
--- a/include/git/tag/TagLoad_Git.class.php
+++ b/include/git/tag/TagLoad_Git.class.php
@@ -110,11 +110,11 @@ class GitPHP_TagLoad_Git implements GitPHP_TagLoadStrategy_Interface
 				$commitHash = $objectHash;
 				break;
 			case 'tag':
 				$args = array();
 				$args[] = 'tag';
-				$args[] = $objectHash;
+				$args[] = escapeshellarg($objectHash);
 				$ret = $this->exe->Execute($tag->GetProject()->GetPath(), GIT_CAT_FILE, $args);
 				$lines = explode("\n", $ret);
 				foreach ($lines as $i => $line) {
 					if (preg_match('/^tag (.+)$/', $line, $regs)) {
 						$name = trim($regs[1]);
diff --git a/include/git/tree/TreeLoad_Base.class.php b/include/git/tree/TreeLoad_Base.class.php
index 0104a52..35814f5 100644
--- a/include/git/tree/TreeLoad_Base.class.php
+++ b/include/git/tree/TreeLoad_Base.class.php
@@ -45,11 +45,11 @@ abstract class GitPHP_TreeLoad_Base implements GitPHP_TreeLoadStrategy_Interface
 
 		$args = array();
 		$args[] = '--full-name';
 		$args[] = '-r';
 		$args[] = '-t';
-		$args[] = $tree->GetHash();
+		$args[] = escapeshellarg($tree->GetHash());
 
 		$lines = explode("\n", $this->exe->Execute($tree->GetProject()->GetPath(), GIT_LS_TREE, $args));
 
 		foreach ($lines as $line) {
 			if (preg_match("/^([0-9]+) (.+) ([0-9a-fA-F]{40})\t(.+)$/", $line, $regs)) {
diff --git a/include/git/tree/TreeLoad_Git.class.php b/include/git/tree/TreeLoad_Git.class.php
index 91a634e..fa12bac 100644
--- a/include/git/tree/TreeLoad_Git.class.php
+++ b/include/git/tree/TreeLoad_Git.class.php
@@ -27,11 +27,11 @@ class GitPHP_TreeLoad_Git extends GitPHP_TreeLoad_Base
 		$args = array();
 		$args[] = '--full-name';
 		if ($this->exe->CanShowSizeInTree())
 			$args[] = '-l';
 		$args[] = '-t';
-		$args[] = $tree->GetHash();
+		$args[] = escapeshellarg($tree->GetHash());
 		
 		$lines = explode("\n", $this->exe->Execute($tree->GetProject()->GetPath(), GIT_LS_TREE, $args));
 
 		foreach ($lines as $line) {
 			if (preg_match("/^([0-9]+) (.+) ([0-9a-fA-F]{40})(\s+[0-9]+|\s+-)?\t(.+)$/", $line, $regs)) {

Vulnerable code: 			$commits = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'rev-list', array('--format=format:"%H %ct"', $head->GetHash())));
			$files = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'ls-tree', array('-r', '--name-only', $head->GetTree()->GetHash())));
		$args[] = $this->commitHash;
		$args[] = $this->path;
				$args[] = '--max-count=' . $this->limit;
				$args[] = '--skip=' . $this->skip;
				$args[] = '--max-count=' . ($this->limit + $this->skip);
		$args[] = $this->path;
		$args[] = $this->path;
		$args[] = '"' . addslashes($this->search) . '"';
		$args[] = $this->treeHash;
			$args[] = $this->fromHash;
		$args[] = $this->toHash;
		$args[] = $blob->GetHash();
		$args[] = $commit->GetHash();
		$args[] = $abbrevHash;
		$args[] = $hash;
		$args[] = '--' . $type;
		$args[] = '--sort=' . $order;
				$args[] = '--count=' . ($count + $skip);
				$args[] = '--count=' . $count;
		$args[] = 'refs/' . $type;
				$extraargs[] = '--max-count=' . $count;
				$extraargs[] = '--skip=' . $skip;
				$extraargs[] = '--max-count=' . ($count + $skip);
				$args[] = $objectHash;
		$args[] = $tree->GetHash();
		$args[] = $tree->GetHash();
Fixed code: 			$commits = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'rev-list', array('--format=format:"%H %ct"', escapeshellarg($head->GetHash()))));
			$files = explode("\n", $this->exe->Execute($this->GetProject()->GetPath(), 'ls-tree', array('-r', '--name-only', escapeshellarg($head->GetTree()->GetHash()))));
		$args[] = escapeshellarg($this->commitHash);
		$args[] = escapeshellarg($this->path);
				$args[] = '--max-count=' . escapeshellarg($this->limit);
				$args[] = '--skip=' . escapeshellarg($this->skip);
				$args[] = '--max-count=' . escapeshellarg($this->limit + $this->skip);
		$args[] = escapeshellarg($this->path);
		$args[] = escapeshellarg($this->path);
		$args[] = escapeshellarg($this->search);
		$args[] = escapeshellarg($this->treeHash);
			$args[] = escapeshellarg($this->fromHash);
		$args[] = escapeshellarg($this->toHash);
		$args[] = escapeshellarg($blob->GetHash());
		$args[] = escapeshellarg($commit->GetHash());
		$args[] = escapeshellarg($abbrevHash);
		$args[] = escapeshellarg($hash);
		$args[] = '--' . escapeshellarg($type);
		$args[] = '--sort=' . escapeshellarg($order);
				$args[] = '--count=' . escapeshellarg($count + $skip);
				$args[] = '--count=' . escapeshellarg($count);
		$args[] = escapeshellarg('refs/' . $type);
				$extraargs[] = '--max-count=' . escapeshellarg($count);
				$extraargs[] = '--skip=' . escapeshellarg($skip);
				$extraargs[] = '--max-count=' . escapeshellarg($count + $skip);
				$args[] = escapeshellarg($objectHash);
		$args[] = escapeshellarg($tree->GetHash());
		$args[] = escapeshellarg($tree->GetHash());