From fc12c0021961f369090a3ea9fdcd62ef4d51505e Mon Sep 17 00:00:00 2001 From: Henry Sudhof Date: Thu, 15 May 2008 14:10:11 +0000 Subject: [PATCH] And more new features for reasonable paranoia. git-svn-id: file:///svn/phpbb/branches/phpBB-3_0_0@8555 89ea8834-ac86-4346-8a33-228a782c2dd0 --- phpBB/docs/CHANGELOG.html | 1 + phpBB/includes/acp/acp_attachments.php | 4 +- phpBB/includes/functions_posting.php | 5 +++ phpBB/includes/functions_upload.php | 59 +++++++++++++++++++++++++- phpBB/includes/functions_user.php | 2 +- phpBB/install/database_update.php | 9 ++-- phpBB/install/schemas/schema_data.sql | 4 +- phpBB/language/en/acp/attachments.php | 2 + phpBB/language/en/common.php | 1 + phpBB/language/en/posting.php | 1 + 10 files changed, 79 insertions(+), 9 deletions(-) diff --git a/phpBB/docs/CHANGELOG.html b/phpBB/docs/CHANGELOG.html index 3e1ae339f9..ef68d5113d 100644 --- a/phpBB/docs/CHANGELOG.html +++ b/phpBB/docs/CHANGELOG.html @@ -105,6 +105,7 @@
  • [Fix] Correctly display subforum read/unread icons from RTL in FF3, Konqueror and Safari3+. (thanks arod-1 for the fix, related to Bug #14830)
  • [Feature] Added optional referer validation of POST requests as additional CSRF protection.
  • [Fix] Added missing form token in acp (thanks NBBN).
    • +
  • [Feature] Added optional stricter upload validation to avoid mime sniffing in addition to the safeguards provided by file.php. (thanks to Nicolas Grekas for compiling the list).

    • 1.ii. Changes since 3.0.0

    diff --git a/phpBB/includes/acp/acp_attachments.php b/phpBB/includes/acp/acp_attachments.php index 53be176924..4e8a8ef719 100644 --- a/phpBB/includes/acp/acp_attachments.php +++ b/phpBB/includes/acp/acp_attachments.php @@ -117,7 +117,9 @@ class acp_attachments 'max_attachments_pm' => array('lang' => 'MAX_ATTACHMENTS_PM', 'validate' => 'int', 'type' => 'text:3:3', 'explain' => false), 'secure_downloads' => array('lang' => 'SECURE_DOWNLOADS', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true), 'secure_allow_deny' => array('lang' => 'SECURE_ALLOW_DENY', 'validate' => 'int', 'type' => 'custom', 'method' => 'select_allow_deny', 'explain' => true), - 'secure_allow_empty_referer' => array('lang' => 'SECURE_EMPTY_REFERRER', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true), + 'secure_allow_empty_referer' => array('lang' => 'SECURE_EMPTY_REFERRER', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true), + 'check_attachment_content' => array('lang' => 'CHECK_CONTENT', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true), + 'legend2' => $l_legend_cat_images, 'img_display_inlined' => array('lang' => 'DISPLAY_INLINED', 'validate' => 'bool', 'type' => 'radio:yes_no', 'explain' => true), diff --git a/phpBB/includes/functions_posting.php b/phpBB/includes/functions_posting.php index 709516a2b0..a7b9cc5bd5 100644 --- a/phpBB/includes/functions_posting.php +++ b/phpBB/includes/functions_posting.php @@ -358,6 +358,11 @@ function upload_attachment($form_name, $forum_id, $local = false, $local_storage include_once($phpbb_root_path . 'includes/functions_upload.' . $phpEx); $upload = new fileupload(); + if ($config['check_attachment_content']) + { + $upload->set_disallowed_content(explode('|', $config['mime_triggers'])); + } + if (!$local) { $filedata['post_attach'] = ($upload->is_valid($form_name)) ? true : false; diff --git a/phpBB/includes/functions_upload.php b/phpBB/includes/functions_upload.php index 50108068cd..a1374b8d54 100644 --- a/phpBB/includes/functions_upload.php +++ b/phpBB/includes/functions_upload.php @@ -228,6 +228,34 @@ class filespec { return @filesize($filename); } + + + /** + * Check the first 256 bytes for forbidden content + */ + function check_content($disallowed_content) + { + if (empty($disallowed_content)) + { + return true; + } + + $fp = @fopen($this->filename, 'rb'); + + if ($fp !== false) + { + $ie_mime_relevant = fread($fp, 256); + fclose($fp); + foreach ($disallowed_content as $forbidden) + { + if (stripos($ie_mime_relevant, '<' . $forbidden) !== false) + { + return false; + } + } + } + return true; + } /** * Move file to destination folder @@ -427,6 +455,7 @@ class fileerror extends filespec class fileupload { var $allowed_extensions = array(); + var $disallowed_content = array(); var $max_filesize = 0; var $min_width = 0; var $min_height = 0; @@ -446,12 +475,13 @@ class fileupload * @param int $max_height Maximum image height (only checked for images) * */ - function fileupload($error_prefix = '', $allowed_extensions = false, $max_filesize = false, $min_width = false, $min_height = false, $max_width = false, $max_height = false) + function fileupload($error_prefix = '', $allowed_extensions = false, $max_filesize = false, $min_width = false, $min_height = false, $max_width = false, $max_height = false, $disallowed_content = false) { $this->set_allowed_extensions($allowed_extensions); $this->set_max_filesize($max_filesize); $this->set_allowed_dimensions($min_width, $min_height, $max_width, $max_height); $this->set_error_prefix($error_prefix); + $this->set_disallowed_content($disallowed_content); } /** @@ -463,6 +493,7 @@ class fileupload $this->min_width = $this->min_height = $this->max_width = $this->max_height = 0; $this->error_prefix = ''; $this->allowed_extensions = array(); + $this->disallowed_content = array(); } /** @@ -497,6 +528,17 @@ class fileupload $this->max_filesize = (int) $max_filesize; } } + + /** + * Set disallowed strings + */ + function set_disallowed_content($disallowed_content) + { + if ($disallowed_content !== false && is_array($disallowed_content)) + { + $this->disallowed_content = $disallowed_content; + } + } /** * Set error prefix @@ -830,6 +872,12 @@ class fileupload { $file->error[] = sprintf($user->lang[$this->error_prefix . 'DISALLOWED_EXTENSION'], $file->get('extension')); } + + // MIME Sniffing + if (!$this->valid_content($file)) + { + $file->error[] = sprintf($user->lang[$this->error_prefix . 'DISALLOWED_CONTENT']); + } } /** @@ -869,6 +917,15 @@ class fileupload return (isset($_FILES[$form_name]) && $_FILES[$form_name]['name'] != 'none') ? true : false; } + + /** + * Check for allowed extension + */ + function valid_content(&$file) + { + return ($file->check_content($this->disallowed_content)); + } + /** * Return image type/extension mapping */ diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index e8414d2ae1..1d774a8ff4 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -1952,7 +1952,7 @@ function avatar_upload($data, &$error) // Init upload class include_once($phpbb_root_path . 'includes/functions_upload.' . $phpEx); - $upload = new fileupload('AVATAR_', array('jpg', 'jpeg', 'gif', 'png'), $config['avatar_filesize'], $config['avatar_min_width'], $config['avatar_min_height'], $config['avatar_max_width'], $config['avatar_max_height']); + $upload = new fileupload('AVATAR_', array('jpg', 'jpeg', 'gif', 'png'), $config['avatar_filesize'], $config['avatar_min_width'], $config['avatar_min_height'], $config['avatar_max_width'], $config['avatar_max_height'], explode('|', $config['mime_triggers'])); if (!empty($_FILES['uploadfile']['name'])) { diff --git a/phpBB/install/database_update.php b/phpBB/install/database_update.php index bead1cad93..94ccf75ecd 100644 --- a/phpBB/install/database_update.php +++ b/phpBB/install/database_update.php @@ -1736,14 +1736,15 @@ function change_database_data($version) _sql($sql, $errored, $error_ary); } } - - // TODO: remove all form token min times break; - - case '3.0.1': + case '3.0.1': + set_config('referer_validation', '1'); + set_config('check_attachment_content', '1'); + set_config('mime_triggers', 'body|head|html|img|plaintext|a href|pre|script|table|title'); + } } diff --git a/phpBB/install/schemas/schema_data.sql b/phpBB/install/schemas/schema_data.sql index 2d417129ed..f8100e36ca 100644 --- a/phpBB/install/schemas/schema_data.sql +++ b/phpBB/install/schemas/schema_data.sql @@ -64,6 +64,7 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('captcha_gd', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('captcha_gd_foreground_noise', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('captcha_gd_x_grid', '25'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('captcha_gd_y_grid', '25'); +INSERT INTO phpbb_config (config_name, config_value) VALUES ('check_attachment_content', '1'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('check_dnsbl', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('chg_passforce', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('cookie_domain', ''); @@ -172,8 +173,7 @@ INSERT INTO phpbb_config (config_name, config_value) VALUES ('max_sig_urls', '5' INSERT INTO phpbb_config (config_name, config_value) VALUES ('min_name_chars', '3'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('min_pass_chars', '6'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('min_search_author_chars', '3'); -INSERT INTO phpbb_config (config_name, config_value) VALUES ('min_time_reg', '0'); -INSERT INTO phpbb_config (config_name, config_value) VALUES ('min_time_terms', '0'); +INSERT INTO phpbb_config (config_name, config_value) VALUES ('mime_triggers', 'body|head|html|img|plaintext|a href|pre|script|table|title'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('override_user_style', '0'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('pass_complex', 'PASS_TYPE_ANY'); INSERT INTO phpbb_config (config_name, config_value) VALUES ('pm_edit_time', '0'); diff --git a/phpBB/language/en/acp/attachments.php b/phpBB/language/en/acp/attachments.php index 6edae1a4cc..594eb9de33 100644 --- a/phpBB/language/en/acp/attachments.php +++ b/phpBB/language/en/acp/attachments.php @@ -71,6 +71,8 @@ $lang = array_merge($lang, array( 'CAT_QUICKTIME_FILES' => 'Quicktime media files', 'CAT_RM_FILES' => 'RealMedia media files', 'CAT_WM_FILES' => 'Windows Media media files', + 'CHECK_CONTENT' => 'Check attachment files', + 'CHECK_CONTENT_EXPLAIN' => 'Some browsers can be tricked to assume an incorrect mimetype for uploaded files. This option ensures that such files likely to cause this are rejected.', 'CREATE_GROUP' => 'Create new group', 'CREATE_THUMBNAIL' => 'Create thumbnail', 'CREATE_THUMBNAIL_EXPLAIN' => 'Create a thumbnail in all possible situations.', diff --git a/phpBB/language/en/common.php b/phpBB/language/en/common.php index db5f49339a..7739c63521 100644 --- a/phpBB/language/en/common.php +++ b/phpBB/language/en/common.php @@ -78,6 +78,7 @@ $lang = array_merge($lang, array( 'ATTACHED_IMAGE_NOT_IMAGE' => 'The image file you tried to attach is invalid.', 'AUTHOR' => 'Author', 'AUTH_NO_PROFILE_CREATED' => 'The creation of a user profile was unsuccessful.', + 'AVATAR_DISALLOWED_CONTENT' => 'The upload was rejected because the uploaded file was identified as a possible attack vector.', 'AVATAR_DISALLOWED_EXTENSION' => 'This file cannot be displayed because the extension %s is not allowed.', 'AVATAR_EMPTY_REMOTE_DATA' => 'The specified avatar could not be uploaded because the remote data appears to be invalid or corrupted.', 'AVATAR_EMPTY_FILEUPLOAD' => 'The uploaded avatar file is empty.', diff --git a/phpBB/language/en/posting.php b/phpBB/language/en/posting.php index 76878c8f7c..f75837e96f 100644 --- a/phpBB/language/en/posting.php +++ b/phpBB/language/en/posting.php @@ -82,6 +82,7 @@ $lang = array_merge($lang, array( 'DISABLE_BBCODE' => 'Disable BBCode', 'DISABLE_MAGIC_URL' => 'Do not automatically parse URLs', 'DISABLE_SMILIES' => 'Disable smilies', + 'DISALLOWED_CONTENT' => 'The upload was rejected because the uploaded file was identified as a possible attack vector.', 'DISALLOWED_EXTENSION' => 'The extension %s is not allowed.', 'DRAFT_LOADED' => 'Draft loaded into posting area, you may want to finish your post now.
    Your draft will be deleted after submitting this post.', 'DRAFT_LOADED_PM' => 'Draft loaded into message area, you may want to finish your private message now.
    Your draft will be deleted after submitting this private message.',