Skip to content

Commit

Permalink
Check for invalid characters in paths
Browse files Browse the repository at this point in the history
  • Loading branch information
icewind1991 committed Aug 17, 2015
1 parent b822af8 commit 4ca3c3d
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 7 deletions.
26 changes: 26 additions & 0 deletions src/AbstractShare.php
Original file line number Diff line number Diff line change
@@ -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');
}
}
}
}
10 changes: 10 additions & 0 deletions src/Exception/InvalidPathException.php
Original file line number Diff line number Diff line change
@@ -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 {}
4 changes: 3 additions & 1 deletion src/NativeShare.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace Icewind\SMB;

class NativeShare implements IShare {
class NativeShare extends AbstractShare {
/**
* @var Server $server
*/
Expand All @@ -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();
Expand Down Expand Up @@ -56,6 +57,7 @@ public function getName() {
}

private function buildUrl($path) {
$this->verifyPath($path);
$url = sprintf('smb://%s/%s', $this->server->getHost(), $this->name);
if ($path) {
$path = trim($path, '/');
Expand Down
8 changes: 3 additions & 5 deletions src/Share.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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()));
Expand Down Expand Up @@ -380,6 +377,7 @@ protected function escape($string) {
* @return string
*/
protected function escapePath($path) {
$this->verifyPath($path);
if ($path === '/') {
$path = '';
}
Expand Down
110 changes: 109 additions & 1 deletion tests/AbstractShare.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

namespace Icewind\SMB\Test;

use Icewind\SMB\Exception\InvalidPathException;
use Icewind\SMB\FileInfo;

abstract class AbstractShare extends TestCase {
Expand Down Expand Up @@ -40,7 +41,6 @@ public function tearDown() {
}

public function nameProvider() {
// / ? < > \ : * | " are illegal characters in path on windows
return array(
array('simple'),
array('with spaces_and-underscores'),
Expand All @@ -53,6 +53,20 @@ public function nameProvider() {
);
}

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'),
Expand Down Expand Up @@ -118,6 +132,18 @@ public function testMkdir($name) {
$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
*/
Expand Down Expand Up @@ -155,6 +181,22 @@ public function testPut($name, $text) {
$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
*/
Expand Down Expand Up @@ -253,6 +295,14 @@ public function testTestRemoveNonExistingFile() {
$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
*/
Expand All @@ -278,6 +328,14 @@ public function testDelFolder() {
$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
*/
Expand All @@ -296,6 +354,14 @@ public function testRmdirNotEmpty() {
$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
*/
Expand All @@ -317,6 +383,14 @@ public function testRenameNonExisting() {
$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
*/
Expand Down Expand Up @@ -350,6 +424,14 @@ public function testReadStream($name, $text) {
$this->assertEquals(file_get_contents($sourceFile), $content);
}

/**
* @dataProvider invalidPathProvider
* @expectedException \Icewind\SMB\Exception\InvalidPathException
*/
public function testReadStreamInvalidPath($name) {
$this->share->read($name);
}

/**
* @dataProvider nameAndDataProvider
*/
Expand All @@ -365,6 +447,16 @@ public function testWriteStream($name, $text) {
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();

Expand Down Expand Up @@ -392,6 +484,14 @@ public function testDir() {
$this->assertFalse($fileEntry->isReadOnly());
}

/**
* @dataProvider invalidPathProvider
* @expectedException \Icewind\SMB\Exception\InvalidPathException
*/
public function testDirInvalidPath($name) {
$this->share->dir($name);
}

/**
* @dataProvider nameProvider
*/
Expand All @@ -406,6 +506,14 @@ public function testStat($name) {
$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
*/
Expand Down

0 comments on commit 4ca3c3d

Please sign in to comment.