Check for invalid characters in paths

src/AbstractShare.php

@@ -0,0 +1,26 @@
+<?php
+/**
+ * Copyright (c) 2015 Robin Appelman <icewind@owncloud.com>
+ * This file is licensed under the Licensed under the MIT license:
+ * http://opensource.org/licenses/MIT
+ */
+
+namespace Icewind\SMB;
+
+use Icewind\SMB\Exception\InvalidPathException;
+
+abstract class AbstractShare implements IShare {
+	private $forbiddenCharacters;
+
+	public function __construct() {
+		$this->forbiddenCharacters = array('?', '<', '>', ':', '*', '|', '"', chr(0), "\n");
+	}
+
+	protected function verifyPath($path) {
+		foreach ($this->forbiddenCharacters as $char) {
+			if (strpos($path, $char) !== false) {
+				throw new InvalidPathException('Invalid path, "' . $char . '" is not allowed');
+			}
+		}
+	}
+}

src/Exception/InvalidPathException.php

@@ -0,0 +1,10 @@
+<?php
+/**
+ * Copyright (c) 2014 Robin Appelman <icewind@owncloud.com>
+ * This file is licensed under the Licensed under the MIT license:
+ * http://opensource.org/licenses/MIT
+ */
+
+namespace Icewind\SMB\Exception;
+
+class InvalidPathException extends InvalidRequestException {}

src/NativeShare.php

@@ -7,7 +7,7 @@
 
 namespace Icewind\SMB;
 
-class NativeShare implements IShare {
+class NativeShare extends AbstractShare {
 	/**
 	 * @var Server $server
 	 */
@@ -28,6 +28,7 @@ class NativeShare implements IShare {
 	 * @param string $name
 	 */
 	public function __construct($server, $name) {
+		parent::__construct();
 		$this->server = $server;
 		$this->name = $name;
 		$this->state = new NativeState();
@@ -56,6 +57,7 @@ class NativeShare implements IShare {
 	}
 
 	private function buildUrl($path) {
+		$this->verifyPath($path);
 		$url = sprintf('smb://%s/%s', $this->server->getHost(), $this->name);
 		if ($path) {
 			$path = trim($path, '/');

src/Share.php

@@ -7,17 +7,13 @@
 
 namespace Icewind\SMB;
 
-use Icewind\SMB\Exception\AccessDeniedException;
-use Icewind\SMB\Exception\AlreadyExistsException;
 use Icewind\SMB\Exception\ConnectionException;
-use Icewind\SMB\Exception\Exception;
 use Icewind\SMB\Exception\FileInUseException;
 use Icewind\SMB\Exception\InvalidTypeException;
-use Icewind\SMB\Exception\NotEmptyException;
 use Icewind\SMB\Exception\NotFoundException;
 use Icewind\Streams\CallbackWrapper;
 
-class Share implements IShare {
+class Share extends AbstractShare {
 	/**
 	 * @var Server $server
 	 */
@@ -43,6 +39,7 @@ class Share implements IShare {
 	 * @param string $name
 	 */
 	public function __construct($server, $name) {
+		parent::__construct();
 		$this->server = $server;
 		$this->name = $name;
 		$this->parser = new Parser(new TimeZoneProvider($this->server->getHost()));
@@ -380,6 +377,7 @@ class Share implements IShare {
 	 * @return string
 	 */
 	protected function escapePath($path) {
+		$this->verifyPath($path);
 		if ($path === '/') {
 			$path = '';
 		}

tests/AbstractShare.php

@@ -7,6 +7,7 @@
 
 namespace Icewind\SMB\Test;
 
+use Icewind\SMB\Exception\InvalidPathException;
 use Icewind\SMB\FileInfo;
 
 abstract class AbstractShare extends TestCase {
@@ -40,7 +41,6 @@ abstract class AbstractShare extends TestCase {
 	}
 
 	public function nameProvider() {
-		// / ? < > \ : * | " are illegal characters in path on windows
 		return array(
 			array('simple'),
 			array('with spaces_and-underscores'),
@@ -53,6 +53,20 @@ abstract class AbstractShare extends TestCase {
 		);
 	}
 
+	public function invalidPathProvider() {
+		// / ? < > \ : * | " are illegal characters in path on windows
+		return array(
+			array("new\nline"),
+			array('null' . chr(0) . 'byte'),
+			array('foo?bar'),
+			array('foo<bar>'),
+			array('foo:bar'),
+			array('foo*bar'),
+			array('foo|bar'),
+			array('foo"bar"')
+		);
+	}
+
 	public function fileDataProvider() {
 		return array(
 			array('Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua'),
@@ -118,6 +132,18 @@ abstract class AbstractShare extends TestCase {
 		$this->assertTrue($dirs[0]->isDirectory());
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testMkdirInvalidPath($name) {
+		$this->share->mkdir($this->root . '/' . $name);
+		$dirs = $this->share->dir($this->root);
+		$this->assertCount(1, $dirs);
+		$this->assertEquals($name, $dirs[0]->getName());
+		$this->assertTrue($dirs[0]->isDirectory());
+	}
+
 	/**
 	 * @dataProvider nameProvider
 	 */
@@ -155,6 +181,22 @@ abstract class AbstractShare extends TestCase {
 		$this->assertFalse($files[0]->isDirectory());
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testPutInvalidPath($name) {
+		$tmpFile = $this->getTextFile('foo');
+
+		try {
+			$this->share->put($tmpFile, $this->root . '/' . $name);
+		} catch (InvalidPathException $e) {
+			unlink($tmpFile);
+			throw $e;
+		}
+		unlink($tmpFile);
+	}
+
 	/**
 	 * @dataProvider nameProvider
 	 */
@@ -253,6 +295,14 @@ abstract class AbstractShare extends TestCase {
 		$this->share->del($this->root . '/foo');
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testDownloadInvalidPath($name) {
+		$this->share->get($name, '');
+	}
+
 	/**
 	 * @expectedException \Icewind\SMB\Exception\NotFoundException
 	 */
@@ -278,6 +328,14 @@ abstract class AbstractShare extends TestCase {
 		$this->share->rmdir($this->root . '/foobar');
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testDelInvalidPath($name) {
+		$this->share->del($name);
+	}
+
 	/**
 	 * @expectedException \Icewind\SMB\Exception\InvalidTypeException
 	 */
@@ -296,6 +354,14 @@ abstract class AbstractShare extends TestCase {
 		$this->share->rmdir($this->root . '/foobar');
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testRmDirInvalidPath($name) {
+		$this->share->rmdir($name);
+	}
+
 	/**
 	 * @expectedException \Icewind\SMB\Exception\NotFoundException
 	 */
@@ -317,6 +383,14 @@ abstract class AbstractShare extends TestCase {
 		$this->share->rename('/foobar/asd', '/foobar/bar');
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testRenameInvalidPath($name) {
+		$this->share->rename($name, $name . '_');
+	}
+
 	/**
 	 * @expectedException \Icewind\SMB\Exception\NotFoundException
 	 */
@@ -350,6 +424,14 @@ abstract class AbstractShare extends TestCase {
 		$this->assertEquals(file_get_contents($sourceFile), $content);
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testReadStreamInvalidPath($name) {
+		$this->share->read($name);
+	}
+
 	/**
 	 * @dataProvider nameAndDataProvider
 	 */
@@ -365,6 +447,16 @@ abstract class AbstractShare extends TestCase {
 		unlink($tmpFile1);
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testWriteStreamInvalidPath($name) {
+		$fh = $this->share->write($this->root . '/' . $name);
+		fwrite($fh, 'foo');
+		fclose($fh);
+	}
+
 	public function testDir() {
 		$txtFile = $this->getTextFile();
 
@@ -392,6 +484,14 @@ abstract class AbstractShare extends TestCase {
 		$this->assertFalse($fileEntry->isReadOnly());
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testDirInvalidPath($name) {
+		$this->share->dir($name);
+	}
+
 	/**
 	 * @dataProvider nameProvider
 	 */
@@ -406,6 +506,14 @@ abstract class AbstractShare extends TestCase {
 		$this->assertEquals($size, $info->getSize());
 	}
 
+	/**
+	 * @dataProvider invalidPathProvider
+	 * @expectedException \Icewind\SMB\Exception\InvalidPathException
+	 */
+	public function testStatInvalidPath($name) {
+		$this->share->stat($name);
+	}
+
 	/**
 	 * @expectedException \Icewind\SMB\Exception\NotFoundException
 	 */