Browse Source

Ignore asset requests containing %2e in them.

urldecode URI's early to avoid issues with encoded `.` characters.
mark_story 12 years ago
parent
commit
d4db64ff26

+ 3 - 3
lib/Cake/Routing/Filter/AssetDispatcher.php

@@ -42,7 +42,7 @@ class AssetDispatcher extends DispatcherFilter {
  * @return CakeResponse if the client is requesting a recognized asset, null otherwise
  * @return CakeResponse if the client is requesting a recognized asset, null otherwise
  */
  */
 	public function beforeDispatch(CakeEvent $event) {
 	public function beforeDispatch(CakeEvent $event) {
-		$url = $event->data['request']->url;
+		$url = urldecode($event->data['request']->url);
 		if (strpos($url, '..') !== false || strpos($url, '.') === false) {
 		if (strpos($url, '..') !== false || strpos($url, '.') === false) {
 			return;
 			return;
 		}
 		}
@@ -118,7 +118,7 @@ class AssetDispatcher extends DispatcherFilter {
 		if ($parts[0] === 'theme') {
 		if ($parts[0] === 'theme') {
 			$themeName = $parts[1];
 			$themeName = $parts[1];
 			unset($parts[0], $parts[1]);
 			unset($parts[0], $parts[1]);
-			$fileFragment = urldecode(implode(DS, $parts));
+			$fileFragment = implode(DS, $parts);
 			$path = App::themePath($themeName) . 'webroot' . DS;
 			$path = App::themePath($themeName) . 'webroot' . DS;
 			return $path . $fileFragment;
 			return $path . $fileFragment;
 		}
 		}
@@ -126,7 +126,7 @@ class AssetDispatcher extends DispatcherFilter {
 		$plugin = Inflector::camelize($parts[0]);
 		$plugin = Inflector::camelize($parts[0]);
 		if ($plugin && CakePlugin::loaded($plugin)) {
 		if ($plugin && CakePlugin::loaded($plugin)) {
 			unset($parts[0]);
 			unset($parts[0]);
-			$fileFragment = urldecode(implode(DS, $parts));
+			$fileFragment = implode(DS, $parts);
 			$pluginWebroot = CakePlugin::path($plugin) . 'webroot' . DS;
 			$pluginWebroot = CakePlugin::path($plugin) . 'webroot' . DS;
 			return $pluginWebroot . $fileFragment;
 			return $pluginWebroot . $fileFragment;
 		}
 		}

+ 44 - 0
lib/Cake/Test/Case/Routing/Filter/AssetDispatcherTest.php

@@ -143,4 +143,48 @@ class AssetDispatcherTest extends CakeTestCase {
 		$this->assertFalse($event->isStopped());
 		$this->assertFalse($event->isStopped());
 	}
 	}
 
 
+/**
+ * Test that attempts to traverse directories are prevented.
+ *
+ * @return void
+ */
+	public function test404OnDoubleDot() {
+		App::build(array(
+			'Plugin' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS),
+			'View' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS)
+		), APP::RESET);
+
+		$response = $this->getMock('CakeResponse', array('_sendHeader'));
+		$request = new CakeRequest('theme/test_theme/../../../../../../VERSION.txt');
+		$event = new CakeEvent('Dispatcher.beforeRequest', $this, compact('request', 'response'));
+
+		$response->expects($this->never())->method('send');
+
+		$filter = new AssetDispatcher();
+		$this->assertNull($filter->beforeDispatch($event));
+		$this->assertFalse($event->isStopped());
+	}
+
+/**
+ * Test that attempts to traverse directories with urlencoded paths fail.
+ *
+ * @return void
+ */
+	public function test404OnDoubleDotEncoded() {
+		App::build(array(
+			'Plugin' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'Plugin' . DS),
+			'View' => array(CAKE . 'Test' . DS . 'test_app' . DS . 'View' . DS)
+		), APP::RESET);
+
+		$response = $this->getMock('CakeResponse', array('_sendHeader', 'send'));
+		$request = new CakeRequest('theme/test_theme/%2e./%2e./%2e./%2e./%2e./%2e./VERSION.txt');
+		$event = new CakeEvent('Dispatcher.beforeRequest', $this, compact('request', 'response'));
+
+		$response->expects($this->never())->method('send');
+
+		$filter = new AssetDispatcher();
+		$this->assertNull($filter->beforeDispatch($event));
+		$this->assertFalse($event->isStopped());
+	}
+
 }
 }