- Patch #693084 by dhthwy, jpmckinney, reglogge, clemens.tolboom, naxoc, chx: file_munge_filename() extension handling broken by move to File Field.

5 files changed, 210 insertions, 13 deletions

diff --git a/modules/aggregator/aggregator.admin.inc b/modules/aggregator/aggregator.admin.inc
index 4deaf6085..a2d964917 100644
--- a/modules/aggregator/aggregator.admin.inc
+++ b/modules/aggregator/aggregator.admin.inc
@@ -300,7 +300,8 @@ function aggregator_form_opml_validate($form, &$form_state) {
  */
 function aggregator_form_opml_submit($form, &$form_state) {
   $data = '';
-  if ($file = file_save_upload('upload')) {
+  $validators = array('file_validate_extensions' => array('opml xml'));
+  if ($file = file_save_upload('upload', $validators)) {
     $data = file_get_contents($file->uri);
   }
   else {
diff --git a/modules/locale/locale.admin.inc b/modules/locale/locale.admin.inc
index be25a35a0..95edf3125 100644
--- a/modules/locale/locale.admin.inc
+++ b/modules/locale/locale.admin.inc
@@ -984,8 +984,9 @@ function locale_translate_import_form($form) {
  * Process the locale import form submission.
  */
 function locale_translate_import_form_submit($form, &$form_state) {
+  $validators = array('file_validate_extensions' => array('po'));
   // Ensure we have the file uploaded
-  if ($file = file_save_upload('file')) {
+  if ($file = file_save_upload('file', $validators)) {
 
     // Add language, if not yet supported
     drupal_static_reset('language_list');
diff --git a/modules/locale/locale.test b/modules/locale/locale.test
index ecc502d90..5b023e381 100644
--- a/modules/locale/locale.test
+++ b/modules/locale/locale.test
@@ -758,7 +758,7 @@ class LocaleImportFunctionalTest extends DrupalWebTestCase {
    *   Additional options to pass to the translation import form.
    */
   function importPoFile($contents, array $options = array()) {
-    $name = tempnam(file_directory_path('temporary'), "po_");
+    $name = tempnam(file_directory_path('temporary'), "po_") . '.po';
     file_put_contents($name, $contents);
     $options['files[file]'] = $name;
     $this->drupalPost('admin/config/regional/translate/import', $options, t('Import'));
@@ -905,7 +905,7 @@ class LocaleExportFunctionalTest extends DrupalWebTestCase {
   function testExportTranslation() {
     // First import some known translations.
     // This will also automatically enable the 'fr' language.
-    $name = tempnam(file_directory_path('temporary'), "po_");
+    $name = tempnam(file_directory_path('temporary'), "po_") . '.po';
     file_put_contents($name, $this->getPoFile());
     $this->drupalPost('admin/config/regional/translate/import', array(
       'langcode' => 'fr',
diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test
index e501f57d0..fe0ebc431 100644
--- a/modules/simpletest/tests/file.test
+++ b/modules/simpletest/tests/file.test
@@ -537,12 +537,17 @@ class FileSaveUploadTest extends FileHookTestCase {
   /**
    * An image file path for uploading.
    */
-  var $image;
+  protected $image;
+
+  /**
+   * A PHP file path for upload security testing.
+   */
+  protected $phpfile;
 
   /**
    * The largest file id when the test starts.
    */
-  var $maxFidBefore;
+  protected $maxFidBefore;
 
   public static function getInfo() {
     return array(
@@ -558,14 +563,17 @@ class FileSaveUploadTest extends FileHookTestCase {
     $this->drupalLogin($account);
 
     $this->image = current($this->drupalGetTestFiles('image'));
-    $this->assertTrue(is_file($this->image->uri), t("The file we're going to upload exists."));
+    $this->assertTrue(is_file($this->image->uri), t("The image file we're going to upload exists."));
+
+    $this->phpfile = current($this->drupalGetTestFiles('php'));
+    $this->assertTrue(is_file($this->phpfile->uri), t("The PHP file we're going to upload exists."));
 
     $this->maxFidBefore = db_query('SELECT MAX(fid) AS fid FROM {file_managed}')->fetchField();
 
-    // Upload with replace to gurantee there's something there.
+    // Upload with replace to guarantee there's something there.
     $edit = array(
       'file_test_replace' => FILE_EXISTS_REPLACE,
-      'files[file_test_upload]' => drupal_realpath($this->image->uri)
+      'files[file_test_upload]' => drupal_realpath($this->image->uri),
     );
     $this->drupalPost('file-test/upload', $edit, t('Submit'));
     $this->assertResponse(200, t('Received a 200 response for posted test file.'));
@@ -631,6 +639,158 @@ class FileSaveUploadTest extends FileHookTestCase {
   }
 
   /**
+   * Test extension handling.
+   */
+  function testHandleExtension() {
+    // The file being tested is a .gif which is in the default safe list
+    // of extensions to allow when the extension validator isn't used. This is
+    // implicitly tested at the testNormal() test. Here we tell
+    // file_save_upload() to only allow ".foo".
+    $extensions = 'foo';
+    $edit = array(
+      'file_test_replace' => FILE_EXISTS_REPLACE,
+      'files[file_test_upload]' => drupal_realpath($this->image->uri),
+      'extensions' => $extensions,
+    );
+
+    $this->drupalPost('file-test/upload', $edit, t('Submit'));
+    $this->assertResponse(200, t('Received a 200 response for posted test file.'));
+    $message = t('Only files with the following extensions are allowed: ') . '<em class="placeholder">' . $extensions . '</em>';
+    $this->assertRaw($message, t('Can\'t upload a disallowed extension'));
+    $this->assertRaw(t('Epic upload FAIL!'), t('Found the failure message.'));
+
+    // Check that the correct hooks were called.
+    $this->assertFileHooksCalled(array('validate'));
+
+    // Reset the hook counters.
+    file_test_reset();
+
+    $extensions = 'foo gif';
+    // Now tell file_save_upload() to allow ".gif".
+    $edit = array(
+      'file_test_replace' => FILE_EXISTS_REPLACE,
+      'files[file_test_upload]' => drupal_realpath($this->image->uri),
+      'extensions' => $extensions,
+    );
+
+    $this->drupalPost('file-test/upload', $edit, t('Submit'));
+    $this->assertResponse(200, t('Received a 200 response for posted test file.'));
+    $this->assertNoRaw(t('Only files with the following extensions are allowed:'), t('Can upload an allowed extension.'));
+    $this->assertRaw(t('You WIN!'), t('Found the success message.'));
+
+    // Check that the correct hooks were called.
+    $this->assertFileHooksCalled(array('validate', 'load', 'update'));
+
+    // Reset the hook counters.
+    file_test_reset();
+
+    // Now tell file_save_upload() to allow any extension.
+    $edit = array(
+      'file_test_replace' => FILE_EXISTS_REPLACE,
+      'files[file_test_upload]' => drupal_realpath($this->image->uri),
+      'allow_all_extensions' => TRUE,
+    );
+    $this->drupalPost('file-test/upload', $edit, t('Submit'));
+    $this->assertResponse(200, t('Received a 200 response for posted test file.'));
+    $this->assertNoRaw(t('Only files with the following extensions are allowed:'), t('Can upload any extension.'));
+    $this->assertRaw(t('You WIN!'), t('Found the success message.'));
+
+    // Check that the correct hooks were called.
+    $this->assertFileHooksCalled(array('validate', 'load', 'update'));
+  }
+
+  /**
+   * Test dangerous file handling.
+   */
+  function testHandleDangerousFile() {
+    // Allow the .php extension and make sure it gets renamed to .txt for
+    // safety. Also check to make sure its MIME type was changed.
+    $edit = array(
+      'file_test_replace' => FILE_EXISTS_REPLACE,
+      'files[file_test_upload]' => drupal_realpath($this->phpfile->uri),
+      'is_image_file' => FALSE,
+      'extensions' => 'php',
+    );
+
+    $this->drupalPost('file-test/upload', $edit, t('Submit'));
+    $this->assertResponse(200, t('Received a 200 response for posted test file.'));
+    $message = t('For security reasons, your upload has been renamed to ') . '<em class="placeholder">' . $this->phpfile->filename . '.txt' . '</em>';
+    $this->assertRaw($message, t('Dangerous file was renamed.'));
+    $this->assertRaw(t('File MIME type is text/plain.'), t('Dangerous file\'s MIME type was changed.'));
+    $this->assertRaw(t('You WIN!'), t('Found the success message.'));
+
+    // Check that the correct hooks were called.
+    $this->assertFileHooksCalled(array('validate', 'insert'));
+
+    // Ensure dangerous files are not renamed when insecure uploads is TRUE.
+    // Turn on insecure uploads.
+    variable_set('allow_insecure_uploads', 1);
+    // Reset the hook counters.
+    file_test_reset();
+
+    $this->drupalPost('file-test/upload', $edit, t('Submit'));
+    $this->assertResponse(200, t('Received a 200 response for posted test file.'));
+    $this->assertNoRaw(t('For security reasons, your upload has been renamed'), t('Found no security message.'));
+    $this->assertRaw(t('File name is !filename', array('!filename' => $this->phpfile->filename)), t('Dangerous file was not renamed when insecure uploads is TRUE.'));
+    $this->assertRaw(t('You WIN!'), t('Found the success message.'));
+
+    // Check that the correct hooks were called.
+    $this->assertFileHooksCalled(array('validate', 'insert'));
+
+    // Turn off insecure uploads.
+    variable_set('allow_insecure_uploads', 0);
+  }
+
+  /**
+   * Test file munge handling.
+   */
+  function testHandleFileMunge() {
+    // Ensure insecure uploads are disabled for this test.
+    variable_set('allow_insecure_uploads', 0);
+    $this->image = file_move($this->image, $this->image->uri . '.foo.gif');
+
+    // Reset the hook counters to get rid of the 'move' we just called.
+    file_test_reset();
+
+    $extensions = 'gif';
+    $edit = array(
+      'files[file_test_upload]' => drupal_realpath($this->image->uri),
+      'extensions' => $extensions,
+    );
+
+    $munged_filename = $this->image->filename;
+    $munged_filename = substr($munged_filename, 0, strrpos($munged_filename, '.'));
+    $munged_filename .= '_.gif';
+
+    $this->drupalPost('file-test/upload', $edit, t('Submit'));
+    $this->assertResponse(200, t('Received a 200 response for posted test file.'));
+    $this->assertRaw(t('For security reasons, your upload has been renamed'), t('Found security message.'));
+    $this->assertRaw(t('File name is !filename', array('!filename' => $munged_filename)), t('File was successfully munged.'));
+    $this->assertRaw(t('You WIN!'), t('Found the success message.'));
+
+    // Check that the correct hooks were called.
+    $this->assertFileHooksCalled(array('validate', 'insert'));
+
+    // Ensure we don't munge files if we're allowing any extension.
+    // Reset the hook counters.
+    file_test_reset();
+
+    $edit = array(
+      'files[file_test_upload]' => drupal_realpath($this->image->uri),
+      'allow_all_extensions' => TRUE,
+    );
+
+    $this->drupalPost('file-test/upload', $edit, t('Submit'));
+    $this->assertResponse(200, t('Received a 200 response for posted test file.'));
+    $this->assertNoRaw(t('For security reasons, your upload has been renamed'), t('Found no security message.'));
+    $this->assertRaw(t('File name is !filename', array('!filename' => $this->image->filename)), t('File was not munged when allowing any extension.'));
+    $this->assertRaw(t('You WIN!'), t('Found the success message.'));
+
+    // Check that the correct hooks were called.
+    $this->assertFileHooksCalled(array('validate', 'insert'));
+  }
+
+  /**
    * Test renaming when uploading over a file that already exists.
    */
   function testExistingRename() {
diff --git a/modules/simpletest/tests/file_test.module b/modules/simpletest/tests/file_test.module
index 6d4d1f8c5..78b80f33d 100644
--- a/modules/simpletest/tests/file_test.module
+++ b/modules/simpletest/tests/file_test.module
@@ -47,7 +47,7 @@ function file_test_stream_wrappers() {
 function _file_test_form($form, &$form_state) {
   $form['file_test_upload'] = array(
     '#type' => 'file',
-    '#title' => t('Upload an image'),
+    '#title' => t('Upload a file'),
   );
   $form['file_test_replace'] = array(
     '#type' => 'select',
@@ -61,9 +61,28 @@ function _file_test_form($form, &$form_state) {
   );
   $form['file_subdir'] = array(
     '#type' => 'textfield',
-    '#title' => 'Subdirectory for test image',
+    '#title' => t('Subdirectory for test file'),
     '#default_value' => '',
   );
+
+  $form['extensions'] = array(
+    '#type' => 'textfield',
+    '#title' => t('Allowed extensions.'),
+    '#default_value' => '',
+  );
+
+  $form['allow_all_extensions'] = array(
+    '#type' => 'checkbox',
+    '#title' => t('Allow all extensions?'),
+    '#default_value' => FALSE,
+  );
+
+  $form['is_image_file'] = array(
+    '#type' => 'checkbox',
+    '#title' => t('Is this an image file?'),
+    '#default_value' => TRUE,
+  );
+
   $form['submit'] = array(
     '#type' => 'submit',
     '#value' => t('Submit'),
@@ -75,7 +94,7 @@ function _file_test_form($form, &$form_state) {
  * Process the upload.
  */
 function _file_test_form_submit(&$form, &$form_state) {
-  // Process the upload and validate that it is an image. Note: we're using the
+  // Process the upload and perform validation. Note: we're using the
   // form value for the $replace parameter.
   if (!empty($form_state['values']['file_subdir'])) {
     $destination = 'temporary://' . $form_state['values']['file_subdir'];
@@ -84,10 +103,26 @@ function _file_test_form_submit(&$form, &$form_state) {
   else {
     $destination = FALSE;
   }
-  $file = file_save_upload('file_test_upload', array('file_validate_is_image' => array()), $destination, $form_state['values']['file_test_replace']);
+
+  // Setup validators.
+  $validators = array();
+  if ($form_state['values']['is_image_file']) {
+    $validators['file_validate_is_image'] = array();
+  }
+
+  if ($form_state['values']['allow_all_extensions']) {
+    $validators['file_validate_extensions'] = array();
+  }
+  else if (!empty($form_state['values']['extensions'])) {
+    $validators['file_validate_extensions'] = array($form_state['values']['extensions']);
+  }
+
+  $file = file_save_upload('file_test_upload', $validators, $destination, $form_state['values']['file_test_replace']);
   if ($file) {
     $form_state['values']['file_test_upload'] = $file;
     drupal_set_message(t('File @filepath was uploaded.', array('@filepath' => $file->uri)));
+    drupal_set_message(t('File name is @filename.', array('@filename' => $file->filename)));
+    drupal_set_message(t('File MIME type is @mimetype.', array('@mimetype' => $file->filemime)));
     drupal_set_message(t('You WIN!'));
   }
   elseif ($file === FALSE) {