Thanks for the detailed report. I confirm the problem, will look into it.

A similar issue (0029135) was reported a few months ago using the same SVG attack vector, except that it refers to CSS injection, but I believe it is the same root cause.

but not all other instances would have CSP implemented

CSP headers are standard in MantisBT and active out of the box on any installation.
As long as source code is not changed or dirty 3rd party plugins change the CSP headers, there is no major security issue.

Of course, there is still an issue when

• using outdated browsers that don't support CSP
• opening the file independant from MantisBT after download

I don't see at the moment how this can be prevented, as changing the content itself when uploading or downloading is not an option.

One more option for MantisBT administrators is to use the following options to configure allowed / non-allowed attachment types.

/**
 * Files that are allowed or not allowed.  Separate items by commas.
 * eg. 'php,html,java,exe,pl'
 * if $g_allowed_files is filled in NO other file types will be allowed.
 * $g_disallowed_files takes precedence over $g_allowed_files
 * @global string $g_allowed_files
 */
$g_allowed_files = '';

/**
 *
 * @global string $g_disallowed_files
 */
$g_disallowed_files = '';

@dregad, we could increase the out of the box security by changing the default settings to something like $g_allowed_files = 'txt';
Not sure if this is a good idea, as it will force most of the admins to change the setting after an upgrade.

I don't see at the moment how this can be prevented, as changing the content itself when uploading or downloading is not an option.

I agree that we should not mess with changing file contents.

My initial idea was to simply add svg to $g_disallowed_files by default, but that does not actually defend against previously uploaded files, and admins who have customized this setting would need to update

So I did some research and found this article https://digi.ninja/blog/svg_xss.php that details the various use cases where users are vulnerable to scripted SVG files. The only one that concerns us is the direct view scenario.

Since we channel all attachments downloads through file_download.php, it should be fairly straightforward to force SVG files to download instead of being displayed, by means of a Content-disposition: attachment header. As it turns out, the script already contains such logic today, but SVG files are currently forced inline so it should be a simple matter of switching the mime type to force them to download instead.

Actually this prevents scripts execution, but an attacker could still play CSS tricks (0029135), so I think we also need to disable the ability to upload SVG to be on the safe side.

CVE Request 1282365 sent

CVE-2022-33910 assigned

@febin attached is a proposed patch for review, thanks in advance for your feedback

diff --git a/config_defaults_inc.php b/config_defaults_inc.php
index 66f3a63aa..eefb01355 100644
--- a/config_defaults_inc.php
+++ b/config_defaults_inc.php
@@ -1976,19 +1976,31 @@ $g_max_file_size = 5 * 1024 * 1024;
 $g_file_upload_max_num = 10;
 
 /**
- * Files that are allowed or not allowed.  Separate items by commas.
- * eg. 'php,html,java,exe,pl'
- * if $g_allowed_files is filled in NO other file types will be allowed.
- * $g_disallowed_files takes precedence over $g_allowed_files
+ * Authorized file types (whitelist).
+ *
+ * If $g_allowed_files is filled in, NO other file types will be allowed. If
+ * empty, any extensions not specifically excluded by $g_disallowed_files list
+ * will be authorized ($g_disallowed_files takes precedence over $g_allowed_files).
+ * Separate items by commas, e.g. 'bmp,gif,jpg,png,txt,zip'.
+ *
+ * @see $g_allowed_files
  * @global string $g_allowed_files
  */
 $g_allowed_files = '';
 
 /**
+ * Forbidden file types (blacklist).
+ *
+ * All file extensions in this list will be unauthorized.
+ * Separate items by commas, e.g. 'php,html,java,exe,pl,svg'.
+ *
+ * SVG files are disabled by default, for security reasons. It is recommended to
+ * also disable all extensions that can be executed by your server;
  *
+ * @see $g_allowed_files
  * @global string $g_disallowed_files
  */
-$g_disallowed_files = '';
+$g_disallowed_files = 'svg';
 
 /**
  * prefix to be used for the file system names of files uploaded to projects.
diff --git a/file_download.php b/file_download.php
index 9ed9b5f44..005fe4d54 100644
--- a/file_download.php
+++ b/file_download.php
@@ -202,9 +202,18 @@ if( $t_content_type_override ) {
 # https://www.thoughtco.com/mime-types-by-content-type-3469108
 $t_show_inline = $f_show_inline;
 $t_mime_force_inline = array(
-	'image/jpeg', 'image/gif', 'image/tiff', 'image/bmp', 'image/svg+xml', 'image/png',
-	'application/pdf' );
-$t_mime_force_attachment = array( 'application/x-shockwave-flash', 'text/html' );
+	'application/pdf',
+	'image/bmp',
+	'image/gif',
+	'image/jpeg',
+	'image/png',
+	'image/tiff',
+);
+$t_mime_force_attachment = array(
+	'application/x-shockwave-flash',
+	'image/svg+xml', # SVG could contain CSS or scripting, see #30384
+	'text/html',
+);
 
 # extract mime type from content type
 $t_mime_type = explode( ';', $t_content_type, 2 );

Svg can include html using <foreignObject> element(works on Firefox only), that can be used for phishing or similar stuff.

Svg can include html using <foreignObject> element(works on Firefox only), that can be used for phishing or similar stuff.

@febin I'm not sure I get your point. The proposed patch completely prevents upload of SVG files by default (can be changed by admin), and as an extra safety measure ensures the SVG files are always downloaded as attachments instead of being opened in a browser tab. I believe other uses cases (display in IMG tags) are not exposed to the vulnerability.

Please clarify and correct me if I'm wrong.