Browse Source

Enforce that relative paths are contained within the template paths.

If user input was used to generate a view path, they could use
a relative path and try to escape the template path directories. Check
the paths if they look like bad things are going to happen.
mark_story 11 years ago
parent
commit
0c7720e8f8
2 changed files with 53 additions and 12 deletions
  1. 24 5
      src/View/View.php
  2. 29 7
      tests/TestCase/View/ViewTest.php

+ 24 - 5
src/View/View.php

@@ -850,22 +850,41 @@ class View {
 					return $name;
 				}
 				$name = trim($name, DS);
-			} elseif ($name[0] === '.') {
-				$name = $this->viewPath . DS . $subDir . $name;
 			} elseif (!$plugin || $this->viewPath !== $this->name) {
 				$name = $this->viewPath . DS . $subDir . $name;
 			}
 		}
-		$paths = $this->_paths($plugin);
-		foreach ($paths as $path) {
+
+		foreach ($this->_paths($plugin) as $path) {
 			if (file_exists($path . $name . $this->_ext)) {
-				return $path . $name . $this->_ext;
+				return $this->_checkFilePath($path . $name . $this->_ext, $path);
 			}
 		}
 		throw new Error\MissingViewException(array('file' => $name . $this->_ext));
 	}
 
 /**
+ * Check that a view file path does not go outside of the defined template paths.
+ *
+ * Only paths that contain `..` will be checked, as they are the ones most likely to
+ * have the ability to resolve to files outside of the template paths.
+ *
+ * @param string $file The path to the template file.
+ * @param string $path Base path that $file should be inside of.
+ * @return string The file path
+ */
+	protected function _checkFilePath($file, $path) {
+		if (strpos($file, '..') === false) {
+			return $file;
+		}
+		$absolute = realpath($file);
+		if (strpos($absolute, $path) !== 0) {
+			throw new Error\MissingViewException(array('file' => $file));
+		}
+		return $absolute;
+	}
+
+/**
  * Splits a dot syntax plugin name into its plugin and filename.
  * If $name does not have a dot, then index 0 will be null.
  * It checks if the plugin is loaded, else filename will stay unchanged for filenames containing dot

+ 29 - 7
tests/TestCase/View/ViewTest.php

@@ -132,12 +132,13 @@ class TestView extends View {
 	}
 
 /**
- * Test only function to return instance scripts.
+ * Setter for extension.
  *
- * @return array Scripts
+ * @param string $ext The extension
+ * @return void
  */
-	public function scripts() {
-		return $this->_scripts;
+	public function ext($ext) {
+		$this->_ext = $ext;
 	}
 
 }
@@ -475,7 +476,8 @@ class ViewTest extends TestCase {
  * @return void
  */
 	public function testGetViewFileNames() {
-		$viewOptions = ['plugin' => null,
+		$viewOptions = [
+			'plugin' => null,
 			'name' => 'Pages',
 			'viewPath' => 'Pages'
 		];
@@ -492,7 +494,7 @@ class ViewTest extends TestCase {
 		$result = $View->getViewFileName('/Posts/index');
 		$this->assertPathEquals($expected, $result);
 
-		$expected = TEST_APP . 'TestApp' . DS . 'Template' . DS . 'Pages' . DS . '..' . DS . 'Posts' . DS . 'index.ctp';
+		$expected = TEST_APP . 'TestApp' . DS . 'Template' . DS . 'Posts' . DS . 'index.ctp';
 		$result = $View->getViewFileName('../Posts/index');
 		$this->assertPathEquals($expected, $result);
 
@@ -513,6 +515,26 @@ class ViewTest extends TestCase {
 	}
 
 /**
+ * Test that getViewFileName() protects against malicious directory traversal.
+ *
+ * @expectedException Cake\View\Error\MissingViewException
+ * @return void
+ */
+	public function testGetViewFileNameDirectoryTraversal() {
+		$viewOptions = [
+			'plugin' => null,
+			'name' => 'Pages',
+			'viewPath' => 'Pages',
+		];
+		$request = $this->getMock('Cake\Network\Request');
+		$response = $this->getMock('Cake\Network\Response');
+
+		$view = new TestView(null, null, null, $viewOptions);
+		$view->ext('.php');
+		$view->getViewFileName('../../../../bootstrap');
+	}
+
+/**
  * Test getting layout filenames
  *
  * @return void
@@ -1127,7 +1149,7 @@ class ViewTest extends TestCase {
 		$result = $View->getViewFileName('../Element/test_element');
 		$this->assertRegExp('/Element(\/|\\\)test_element.ctp/', $result);
 
-		$expected = TEST_APP . 'TestApp' . DS . 'Template' . DS . 'Posts' . DS . '..' . DS . 'Posts' . DS . 'index.ctp';
+		$expected = TEST_APP . 'TestApp' . DS . 'Template' . DS . 'Posts' . DS . 'index.ctp';
 		$result = $View->getViewFileName('../Posts/index');
 		$this->assertPathEquals($expected, $result);
 	}