CVE-2015-7698: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: icewind1991 SMB before 1.0.3 allows remote authenticated users to execute arbitrary SMB commands via shell metacharacters in the user argument in the (1) listShares function in Server.php or the (2) connect or (3) read function in Share.php.
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.
Published: 2015-10-21T18:59Z
Repository: https://github.com/icewind1991/SMB
Language: PHP
Files changed: src/NativeServer.php, src/NativeShare.php, src/Server.php, src/Share.php
Commit date: 2015-08-13 16:19:03 +0200
Commit message: improve support for workgroups/domains

Diff: diff --git a/src/NativeServer.php b/src/NativeServer.php
index aeb5d3a..ce8e6d6 100644
--- a/src/NativeServer.php
+++ b/src/NativeServer.php
@@ -22,16 +22,11 @@ class NativeServer extends Server {
 		parent::__construct($host, $user, $password);
 		$this->state = new NativeState();
 	}
 
 	protected function connect() {
-		$user = $this->getUser();
-		$workgroup = null;
-		if (strpos($user, '/')) {
-			list($workgroup, $user) = explode('/', $user);
-		}
-		$this->state->init($workgroup, $user, $this->getPassword());
+		$this->state->init($this->getWorkgroup(), $this->getUser(), $this->getPassword());
 	}
 
 	/**
 	 * @return \Icewind\SMB\IShare[]
 	 * @throws \Icewind\SMB\Exception\AuthenticationException
diff --git a/src/NativeShare.php b/src/NativeShare.php
index 044dd7a..ecef8c7 100644
--- a/src/NativeShare.php
+++ b/src/NativeShare.php
@@ -41,19 +41,11 @@ class NativeShare implements IShare {
 	protected function connect() {
 		if ($this->state and $this->state instanceof NativeShare) {
 			return;
 		}
 
-		$user = $this->server->getUser();
-		if (strpos($user, '/')) {
-			list($workgroup, $user) = explode('/', $user);
-		} elseif (strpos($user, '\\')) {
-			list($workgroup, $user) = explode('\\', $user);
-		} else {
-			$workgroup = null;
-		}
-		$this->state->init($workgroup, $user, $this->server->getPassword());
+		$this->state->init($this->server->getWorkgroup(), $this->server->getUser(), $this->server->getPassword());
 	}
 
 	/**
 	 * Get the name of the share
 	 *
diff --git a/src/Server.php b/src/Server.php
index f7227d4..5c07d1c 100644
--- a/src/Server.php
+++ b/src/Server.php
@@ -27,10 +27,15 @@ class Server {
 	/**
 	 * @var string $password
 	 */
 	protected $password;
 
+	/**
+	 * @var string $workgroup
+	 */
+	protected $workgroup;
+
 	/**
 	 * Check if the smbclient php extension is available
 	 *
 	 * @return bool
 	 */
@@ -43,14 +48,32 @@ class Server {
 	 * @param string $user
 	 * @param string $password
 	 */
 	public function __construct($host, $user, $password) {
 		$this->host = $host;
+		list($workgroup, $user) = $this->splitUser($user);
 		$this->user = $user;
+		$this->workgroup = $workgroup;
 		$this->password = $password;
 	}
 
+	/**
+	 * Split workgroup from username
+	 *
+	 * @param $user
+	 * @return string[] [$workgroup, $user]
+	 */
+	public function splitUser($user) {
+		if (strpos($user, '/')) {
+			return explode('/', $user, 2);
+		} elseif (strpos($user, '\\')) {
+			return explode('\\', $user);
+		} else {
+			return [null, $user];
+		}
+	}
+
 	/**
 	 * @return string
 	 */
 	public function getAuthString() {
 		return $this->user . '%' . $this->password;
@@ -75,18 +98,26 @@ class Server {
 	 */
 	public function getHost() {
 		return $this->host;
 	}
 
+	/**
+	 * @return string
+	 */
+	public function getWorkgroup() {
+		return $this->workgroup;
+	}
+
 	/**
 	 * @return \Icewind\SMB\IShare[]
 	 *
 	 * @throws \Icewind\SMB\Exception\AuthenticationException
 	 * @throws \Icewind\SMB\Exception\InvalidHostException
 	 */
 	public function listShares() {
-		$command = Server::CLIENT . ' --authentication-file=/proc/self/fd/3' .
+		$workgroupArgument = ($this->workgroup) ? ' -W ' . escapeshellarg($this->workgroup) : '';
+		$command = Server::CLIENT . $workgroupArgument . ' --authentication-file=/proc/self/fd/3' .
 			' -gL ' . escapeshellarg($this->getHost());
 		$connection = new RawConnection($command);
 		$connection->writeAuthentication($this->getUser(), $this->getPassword());
 		$output = $connection->readAll();
 
diff --git a/src/Share.php b/src/Share.php
index 7c24f9f..8657e69 100644
--- a/src/Share.php
+++ b/src/Share.php
@@ -55,12 +55,14 @@ class Share implements IShare {
 	 */
 	protected function connect() {
 		if ($this->connection and $this->connection->isValid()) {
 			return;
 		}
-		$command = sprintf('%s --authentication-file=/proc/self/fd/3 //%s/%s',
+		$workgroupArgument = ($this->server->getWorkgroup()) ? ' -W ' . escapeshellarg($this->server->getWorkgroup()) : '';
+		$command = sprintf('%s %s --authentication-file=/proc/self/fd/3 //%s/%s',
 			Server::CLIENT,
+			$workgroupArgument,
 			$this->server->getHost(),
 			$this->name
 		);
 		$this->connection = new Connection($command);
 		$this->connection->writeAuthentication($this->server->getUser(), $this->server->getPassword());
@@ -258,12 +260,14 @@ class Share implements IShare {
 		$source = $this->escapePath($source);
 		// close the single quote, open a double quote where we put the single quote...
 		$source = str_replace('\'', '\'"\'"\'', $source);
 		// since returned stream is closed by the caller we need to create a new instance
 		// since we can't re-use the same file descriptor over multiple calls
-		$command = sprintf('%s --authentication-file=/proc/self/fd/3 //%s/%s -c \'get %s /proc/self/fd/5\'',
+		$workgroupArgument = ($this->server->getWorkgroup()) ? ' -W ' . escapeshellarg($this->server->getWorkgroup()) : '';
+		$command = sprintf('%s %s --authentication-file=/proc/self/fd/3 //%s/%s -c \'get %s /proc/self/fd/5\'',
 			Server::CLIENT,
+			$workgroupArgument,
 			$this->server->getHost(),
 			$this->name,
 			$source
 		);
 		$connection = new Connection($command);
@@ -286,12 +290,14 @@ class Share implements IShare {
 		$target = $this->escapePath($target);
 		// close the single quote, open a double quote where we put the single quote...
 		$target = str_replace('\'', '\'"\'"\'', $target);
 		// since returned stream is closed by the caller we need to create a new instance
 		// since we can't re-use the same file descriptor over multiple calls
-		$command = sprintf('%s --authentication-file=/proc/self/fd/3 //%s/%s -c \'put /proc/self/fd/4 %s\'',
+		$workgroupArgument = ($this->server->getWorkgroup()) ? ' -W ' . escapeshellarg($this->server->getWorkgroup()) : '';
+		$command = sprintf('%s %s --authentication-file=/proc/self/fd/3 //%s/%s -c \'put /proc/self/fd/4 %s\'',
 			Server::CLIENT,
+			$workgroupArgument,
 			$this->server->getHost(),
 			$this->name,
 			$target
 		);
 		$connection = new RawConnection($command);

Vulnerable code: 		$user = $this->getUser();
		$workgroup = null;
		if (strpos($user, '/')) {
			list($workgroup, $user) = explode('/', $user);
		}
		$this->state->init($workgroup, $user, $this->getPassword());
		$user = $this->server->getUser();
		if (strpos($user, '/')) {
			list($workgroup, $user) = explode('/', $user);
		} elseif (strpos($user, '\\')) {
			list($workgroup, $user) = explode('\\', $user);
		} else {
			$workgroup = null;
		}
		$this->state->init($workgroup, $user, $this->server->getPassword());
		$command = Server::CLIENT . ' --authentication-file=/proc/self/fd/3' .
		$command = sprintf('%s --authentication-file=/proc/self/fd/3 //%s/%s',
		$command = sprintf('%s --authentication-file=/proc/self/fd/3 //%s/%s -c \'get %s /proc/self/fd/5\'',
		$command = sprintf('%s --authentication-file=/proc/self/fd/3 //%s/%s -c \'put /proc/self/fd/4 %s\'',
Fixed code: 		$this->state->init($this->getWorkgroup(), $this->getUser(), $this->getPassword());
		$this->state->init($this->server->getWorkgroup(), $this->server->getUser(), $this->server->getPassword());
	/**
	 * @var string $workgroup
	 */
	protected $workgroup;

		list($workgroup, $user) = $this->splitUser($user);
		$this->workgroup = $workgroup;
	/**
	 * Split workgroup from username
	 *
	 * @param $user
	 * @return string[] [$workgroup, $user]
	 */
	public function splitUser($user) {
		if (strpos($user, '/')) {
			return explode('/', $user, 2);
		} elseif (strpos($user, '\\')) {
			return explode('\\', $user);
		} else {
			return [null, $user];
		}
	}

	/**
	 * @return string
	 */
	public function getWorkgroup() {
		return $this->workgroup;
	}

		$workgroupArgument = ($this->workgroup) ? ' -W ' . escapeshellarg($this->workgroup) : '';
		$command = Server::CLIENT . $workgroupArgument . ' --authentication-file=/proc/self/fd/3' .
		$workgroupArgument = ($this->server->getWorkgroup()) ? ' -W ' . escapeshellarg($this->server->getWorkgroup()) : '';
		$command = sprintf('%s %s --authentication-file=/proc/self/fd/3 //%s/%s',
			$workgroupArgument,
		$workgroupArgument = ($this->server->getWorkgroup()) ? ' -W ' . escapeshellarg($this->server->getWorkgroup()) : '';
		$command = sprintf('%s %s --authentication-file=/proc/self/fd/3 //%s/%s -c \'get %s /proc/self/fd/5\'',
			$workgroupArgument,
		$workgroupArgument = ($this->server->getWorkgroup()) ? ' -W ' . escapeshellarg($this->server->getWorkgroup()) : '';
		$command = sprintf('%s %s --authentication-file=/proc/self/fd/3 //%s/%s -c \'put /proc/self/fd/4 %s\'',
			$workgroupArgument,