From 1c3cb7fdbb21f9e7cbbe7637b473bcd540e11e37 Mon Sep 17 00:00:00 2001
From: Damien Regad <dregad@mantisbt.org>
Date: Fri, 28 Nov 2014 17:35:27 -0800
Subject: [PATCH] Fix XSS in file uploads

An attacker can upload a Flash file with an image extension. If such an
attachment is displayed inline, it becomes a vector for XSS attacks.

This issue was reported by Matthias Karlsson (http://mathiaskarlsson.me)
as part of Offensive Security's bug bounty program [1].

Fixes #17874
---
 file_download.php | 81 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 38 insertions(+), 43 deletions(-)

diff --git a/file_download.php b/file_download.php
index 07fcdf2..12d0742 100644
--- a/file_download.php
+++ b/file_download.php
@@ -130,14 +130,10 @@
 
 	header( 'Last-Modified: ' . gmdate( 'D, d M Y H:i:s \G\M\T', $v_date_added ) );
 
+	$t_upload_method = config_get( 'file_upload_method' );
 	$t_filename = file_get_display_name( $v_filename );
-	# For Internet Explorer 8 as per http://blogs.msdn.com/ie/archive/2008/07/02/ie8-security-part-v-comprehensive-protection.aspx
-	# Don't let IE second guess our content-type!
-	header( 'X-Content-Type-Options: nosniff' );
 
-	http_content_disposition_header( $t_filename, $f_show_inline );
-
-	header( 'Content-Length: ' . $v_filesize );
+	# Content headers
 
 	# If finfo is available (always true for PHP >= 5.3.0) we can use it to determine the MIME type of files
 	$finfo = finfo_get_if_available();
@@ -145,27 +141,13 @@
 	$t_content_type = $v_file_type;
 
 	$t_content_type_override = file_get_content_type_override ( $t_filename );
+	$t_file_info_type = false;
 
-	# dump file content to the connection.
-	switch ( config_get( 'file_upload_method' ) ) {
+	switch( $t_upload_method ) {
 		case DISK:
 			$t_local_disk_file = file_normalize_attachment_path( $v_diskfile, $t_project_id );
-
-			if ( file_exists( $t_local_disk_file ) ) {
-				if ( $finfo ) {
-					$t_file_info_type = $finfo->file( $t_local_disk_file );
-
-					if ( $t_file_info_type !== false ) {
-						$t_content_type = $t_file_info_type;
-					}
-				}
-
-				if ( $t_content_type_override ) {
-					$t_content_type = $t_content_type_override;
-				}
-
-				header( 'Content-Type: ' . $t_content_type );
-				readfile( $t_local_disk_file );
+			if( file_exists( $t_local_disk_file ) && $finfo ) {
+				$t_file_info_type = $finfo->file( $t_local_disk_file );
 			}
 			break;
 		case FTP:
@@ -179,32 +161,45 @@
 
 			if ( $finfo ) {
 				$t_file_info_type = $finfo->file( $t_local_disk_file );
-
-				if ( $t_file_info_type !== false ) {
-					$t_content_type = $t_file_info_type;
-				}
 			}
-
-			if ( $t_content_type_override ) {
-				$t_content_type = $t_content_type_override;
-			}
-
-			header( 'Content-Type: ' . $t_content_type );
-			readfile( $t_local_disk_file );
 			break;
+		case DATABASE:
 		default:
 			if ( $finfo ) {
 				$t_file_info_type = $finfo->buffer( $v_content );
-
-				if ( $t_file_info_type !== false ) {
-					$t_content_type = $t_file_info_type;
-				}
 			}
+	}
 
-			if ( $t_content_type_override ) {
-				$t_content_type = $t_content_type_override;
-			}
+	if ( $t_file_info_type !== false ) {
+		$t_content_type = $t_file_info_type;
 
-			header( 'Content-Type: ' . $t_content_type );
+		# Don't allow inline flash
+		if( false !== strpos( $t_file_info_type, 'application/x-shockwave-flash' ) ) {
+			http_content_disposition_header( $t_filename );
+		} else {
+			http_content_disposition_header( $t_filename, $f_show_inline );
+		}
+	}
+
+	if ( $t_content_type_override ) {
+		$t_content_type = $t_content_type_override;
+	}
+
+	header( 'Content-Type: ' . $t_content_type );
+	header( 'Content-Length: ' . $v_filesize );
+
+	# For Internet Explorer 8 as per http://blogs.msdn.com/ie/archive/2008/07/02/ie8-security-part-v-comprehensive-protection.aspx
+	# Don't let IE second guess our content-type!
+	header( 'X-Content-Type-Options: nosniff' );
+
+	# dump file content to the connection.
+	switch( $t_upload_method ) {
+		case DISK:
+		case FTP:
+			readfile( $t_local_disk_file );
+			break;
+
+		case DATABASE:
+		default:
 			echo $v_content;
 	}
-- 
1.9.3 (Apple Git-50)