From 519adb506073f4c3e2afd57e6ba1b6da4569e606 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 3 Apr 2014 15:55:18 +0200 Subject: [PATCH 1/3] [ticket/10851] Set disallowed content to empty array if checking is disabled The disallowed content defaults to a standard set of mimetype triggers by default. If one doesn't want to check the attachments mimetype triggers then we need to set the disallowed content to an empty array. PHPBB3-10851 --- phpBB/includes/functions_posting.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index 11a5067ef9..3f0a78a7cb 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -394,6 +394,10 @@ function upload_attachment($form_name, $forum_id, $local = false, $local_storage { $upload->set_disallowed_content(explode('|', $config['mime_triggers'])); } + else if (!$config['check_attachment_content']) + { + $upload->set_disallowed_content(array()); + } if (!$local) { From 91c3ad07eca3c755fd3be839af2956457e05d10f Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Thu, 3 Apr 2014 18:26:39 +0200 Subject: [PATCH 2/3] [ticket/10851] Add unit and functional tests for checking attachments These tests will make sure that fileuploads now work as expected regarding the check for disallowed content. PHPBB3-10851 --- tests/functional/fileupload_form_test.php | 33 ++++++++++++++++++ .../functional/fixtures/files/disallowed.jpg | Bin 0 -> 559 bytes tests/upload/filespec_test.php | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 tests/functional/fixtures/files/disallowed.jpg diff --git a/tests/functional/fileupload_form_test.php b/tests/functional/fileupload_form_test.php index c291712c71..eaa6d634a5 100644 --- a/tests/functional/fileupload_form_test.php +++ b/tests/functional/fileupload_form_test.php @@ -76,6 +76,39 @@ class phpbb_functional_fileupload_form_test extends phpbb_functional_test_case $this->assertEquals($this->lang('DISALLOWED_EXTENSION', 'bif'), $crawler->filter('p.error')->text()); } + public function test_disallowed_content() + { + $this->login(); + + $crawler = $this->upload_file('disallowed.jpg', 'image/jpeg'); + $this->assertEquals($this->lang('DISALLOWED_CONTENT'), $crawler->filter('p.error')->text()); + } + + public function test_disallowed_content_no_check() + { + $this->login(); + $this->admin_login(); + $this->add_lang('ucp'); + $crawler = self::request('GET', 'adm/index.php?sid=' . $this->sid . '&i=acp_attachments&mode=attach'); + + $form = $crawler->selectButton('Submit')->form(); + $values = $form->getValues(); + + $values["config[check_attachment_content]"] = 0; + $form->setValues($values); + $crawler = self::submit($form); + + // Logout and back in for correct URL + $this->logout(); + $this->login(); + + $crawler = $this->upload_file('disallowed.jpg', 'image/jpeg'); + + // Hitting the UNABLE_GET_IMAGE_SIZE error means we passed the + // DISALLOWED_CONTENT check + $this->assertEquals($this->lang('UNABLE_GET_IMAGE_SIZE'), $crawler->filter('p.error')->text()); + } + public function test_too_large() { $this->create_user('fileupload'); diff --git a/tests/functional/fixtures/files/disallowed.jpg b/tests/functional/fixtures/files/disallowed.jpg new file mode 100644 index 0000000000000000000000000000000000000000..06a437585a5767be852cf841a8a325e3b52c0cf6 GIT binary patch literal 559 zcmbtPK?=e!5ZuY4^x$CwdKSOXb1{cPt)k$;|G&lA1bT=x9y)=r+04$&iV*gg?vntJ zna42>^XK)v=6d5bVxxeM$4+|b6;VQjMwdJ>aHSmhKK^u_bWlhzHIaITpXy^)f4tf4 yD283QIU$>}SWpNGB(w-&A*@>LQ?XbjAYI%dZ;|72ks4<J7jE literal 0 HcmV?d00001 diff --git a/tests/upload/filespec_test.php b/tests/upload/filespec_test.php index 87cd00197f..492f31cee6 100644 --- a/tests/upload/filespec_test.php +++ b/tests/upload/filespec_test.php @@ -143,6 +143,8 @@ class phpbb_filespec_test extends phpbb_test_case $disallowed_content = explode('|', $this->config['mime_triggers']); $filespec = $this->get_filespec(array('tmp_name' => $this->path . $filename)); $this->assertEquals($expected, $filespec->check_content($disallowed_content)); + // All files should pass if $disallowed_content is empty + $this->assertEquals(true, $filespec->check_content(array())); } public function clean_filename_variables() From fdfaba4607be827a63648c5103e84b96c8e85290 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 4 Apr 2014 20:55:29 +0200 Subject: [PATCH 3/3] [ticket/10851] Request index instead of logging in and out in tests PHPBB3-10851 --- tests/functional/fileupload_form_test.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/functional/fileupload_form_test.php b/tests/functional/fileupload_form_test.php index eaa6d634a5..30f6fd7b47 100644 --- a/tests/functional/fileupload_form_test.php +++ b/tests/functional/fileupload_form_test.php @@ -98,9 +98,8 @@ class phpbb_functional_fileupload_form_test extends phpbb_functional_test_case $form->setValues($values); $crawler = self::submit($form); - // Logout and back in for correct URL - $this->logout(); - $this->login(); + // Request index for correct URL + $crawler = self::request('GET', 'index.php?sid=' . $this->sid); $crawler = $this->upload_file('disallowed.jpg', 'image/jpeg');