Fix possibility for spoofed files to pass validation.

Use `is_uploaded_file` to prevent crafty requests that contain bogus files from getting through. A testing stub class was necessary to avoid making significant changes to the test suite.
cakephp · Mar 29, 2016 · 1926d40 · 1926d40
1 parent 24df4dd
commit 1926d40
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 13 deletions.
diff --git a/lib/Cake/Test/Case/Utility/ValidationTest.php b/lib/Cake/Test/Case/Utility/ValidationTest.php
@@ -88,6 +88,25 @@ public static function phone($check) {
 
 }
 
+/**
+ * ValidationStub
+ *
+ * @package       Cake.Test.Case.Utility
+ */
+class ValidationStub extends Validation {
+
+/**
+ * Stub out is_uploaded_file check
+ *
+ * @param string $path
+ * @return void
+ */
+	protected static function _isUploadedFile($path) {
+		return file_exists($path);
+	}
+
+}
+
 /**
  * Test Case for Validation Class
  *
@@ -2405,21 +2424,21 @@ public function testFileSize() {
  * @return void
  */
 	public function testUploadedFileErrorCode() {
-		$this->assertFalse(Validation::uploadedFile('derp'));
+		$this->assertFalse(ValidationStub::uploadedFile('derp'));
 		$invalid = array(
 			'name' => 'testing'
 		);
-		$this->assertFalse(Validation::uploadedFile($invalid));
+		$this->assertFalse(ValidationStub::uploadedFile($invalid));
 		$file = array(
 			'name' => 'cake.power.gif',
 			'tmp_name' => CORE_PATH . 'Cake' . DS . 'Test' . DS . 'test_app' . DS . 'webroot/img/cake.power.gif',
 			'error' => UPLOAD_ERR_OK,
 			'type' => 'image/gif',
 			'size' => 201
 		);
-		$this->assertTrue(Validation::uploadedFile($file));
+		$this->assertTrue(ValidationStub::uploadedFile($file));
 		$file['error'] = UPLOAD_ERR_NO_FILE;
-		$this->assertFalse(Validation::uploadedFile($file), 'Error upload should fail.');
+		$this->assertFalse(ValidationStub::uploadedFile($file), 'Error upload should fail.');
 	}
 
 /**
@@ -2438,11 +2457,11 @@ public function testUploadedFileMimeType() {
 		$options = array(
 			'types' => array('text/plain')
 		);
-		$this->assertFalse(Validation::uploadedFile($file, $options), 'Incorrect mimetype.');
+		$this->assertFalse(ValidationStub::uploadedFile($file, $options), 'Incorrect mimetype.');
 		$options = array(
 			'types' => array('image/gif', 'image/png')
 		);
-		$this->assertTrue(Validation::uploadedFile($file, $options));
+		$this->assertTrue(ValidationStub::uploadedFile($file, $options));
 	}
 
 /**
@@ -2461,24 +2480,24 @@ public function testUploadedFileSize() {
 		$options = array(
 			'minSize' => 500
 		);
-		$this->assertFalse(Validation::uploadedFile($file, $options), 'Too small');
+		$this->assertFalse(ValidationStub::uploadedFile($file, $options), 'Too small');
 		$options = array(
 			'maxSize' => 100
 		);
-		$this->assertFalse(Validation::uploadedFile($file, $options), 'Too big');
+		$this->assertFalse(ValidationStub::uploadedFile($file, $options), 'Too big');
 		$options = array(
 			'minSize' => 100,
 		);
-		$this->assertTrue(Validation::uploadedFile($file, $options));
+		$this->assertTrue(ValidationStub::uploadedFile($file, $options));
 		$options = array(
 			'maxSize' => 500,
 		);
-		$this->assertTrue(Validation::uploadedFile($file, $options));
+		$this->assertTrue(ValidationStub::uploadedFile($file, $options));
 		$options = array(
 			'minSize' => 100,
 			'maxSize' => 500
 		);
-		$this->assertTrue(Validation::uploadedFile($file, $options));
+		$this->assertTrue(ValidationStub::uploadedFile($file, $options));
 	}
 
 /**
@@ -2519,6 +2538,6 @@ public function testUploadedFileWithDifferentFileParametersOrder() {
 			'size' => 201
 		);
 		$options = array();
-		$this->assertTrue(Validation::uploadedFile($file, $options), 'Wrong order');
+		$this->assertTrue(ValidationStub::uploadedFile($file, $options), 'Wrong order');
 	}
 }
diff --git a/lib/Cake/Utility/Validation.php b/lib/Cake/Utility/Validation.php
@@ -1036,7 +1036,17 @@ public static function uploadedFile($file, $options = array()) {
 		if (isset($options['types']) && !static::mimeType($file, $options['types'])) {
 			return false;
 		}
-		return true;
+		return static::_isUploadedFile($file['tmp_name']);
+	}
+
+/**
+ * Helper method that can be stubbed in testing.
+ *
+ * @param string $path The path to check.
+ * @return bool Whether or not the file is an uploaded file.
+ */
+	protected static function _isUploadedFile($path) {
+		return is_uploaded_file($path);
 	}
 
 /**