From: lpsolit%gmail.com <> Date: Mon, 2 Feb 2009 19:13:52 +0000 (+0000) Subject: Bug 472206: [SECURITY] Bugzilla should optionally not allow the user to view possibly... X-Git-Tag: bugzilla-3.0.7~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0d8a3e2c2ffb5ff95a7fb96ed04c03dc43284eca;p=thirdparty%2Fbugzilla.git Bug 472206: [SECURITY] Bugzilla should optionally not allow the user to view possibly harmful attachments - Patch by Frédéric Buclin r=mkanat a=LpSolit --- diff --git a/Bugzilla/Config/Attachment.pm b/Bugzilla/Config/Attachment.pm index d498157f9b..15ba2672ae 100644 --- a/Bugzilla/Config/Attachment.pm +++ b/Bugzilla/Config/Attachment.pm @@ -40,7 +40,13 @@ $Bugzilla::Config::Attachment::sortkey = "025"; sub get_param_list { my $class = shift; my @param_list = ( - { + { + name => 'allow_attachment_display', + type => 'b', + default => 0 + }, + + { name => 'attachment_base', type => 't', default => '', diff --git a/attachment.cgi b/attachment.cgi index 25b73828e5..53fe4a6d0c 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -390,8 +390,10 @@ sub view { $filename =~ s/\\/\\\\/g; # escape backslashes $filename =~ s/"/\\"/g; # escape quotes + my $disposition = Bugzilla->params->{'allow_attachment_display'} ? 'inline' : 'attachment'; + print $cgi->header(-type=>"$contenttype; name=\"$filename\"", - -content_disposition=> "inline; filename=\"$filename\"", + -content_disposition=> "$disposition; filename=\"$filename\"", -content_length => $attachment->datasize); print $attachment->data; diff --git a/template/en/default/admin/params/attachment.html.tmpl b/template/en/default/admin/params/attachment.html.tmpl index 721177429e..50c6ad606d 100644 --- a/template/en/default/admin/params/attachment.html.tmpl +++ b/template/en/default/admin/params/attachment.html.tmpl @@ -25,23 +25,38 @@ %] [% param_descs = { - attachment_base => "It is possible for a malicious attachment to steal your " _ - "cookies or access other attachments to perform an attack " _ - "on the user.

" _ - "If you would like additional security on attachments " _ - "to avoid this, set this parameter to an alternate URL " _ - "for your $terms.Bugzilla that is not the same as " _ - "urlbase or sslbase. That is, a different " _ - "domain name that resolves to this exact same $terms.Bugzilla " _ - "installation.

" _ - "For added security, you can insert %bugid% into " _ - "the URL, which will be replaced with the ID of the current " _ - "$terms.bug that the attachment is on, when you access " _ - "an attachment. This will limit attachments to accessing " _ - "only other attachments on the same ${terms.bug}. " _ - "Remember, though, that all those possible domain names " _ - "(such as 1234.your.domain.com) must point to " _ - "this same $terms.Bugzilla instance." + allow_attachment_display => + "If this option is on, users will be able to view attachments from" + _ " their browser, if their browser supports the attachment's MIME type." + _ " If this option is off, users are forced to download attachments," + _ " even if the browser is able to display them." + _ "

This is a security restriction for installations where untrusted" + _ " users may upload attachments that could be potentially damaging if" + _ " viewed directly in the browser.

" + _ "

It is highly recommended that you set the attachment_base" + _ " parameter if you turn this parameter on.", + + attachment_base => + "When the allow_attachment_display parameter is on, it is " + _ " possible for a malicious attachment to steal your cookies or" + _ " perform an attack on $terms.Bugzilla using your credentials." + _ "

If you would like additional security on attachments to avoid" + _ " this, set this parameter to an alternate URL for your $terms.Bugzilla" + _ " that is not the same as urlbase or sslbase." + _ " That is, a different domain name that resolves to this exact" + _ " same $terms.Bugzilla installation.

" + _ "

Note that if you have set the" + _ " cookiedomain" + _" parameter, you should set attachment_base to use a" + _ " domain that would not be matched by" + _ " cookiedomain.

" + _ "

For added security, you can insert %bugid% into the URL," + _ " which will be replaced with the ID of the current $terms.bug that" + _ " the attachment is on, when you access an attachment. This will limit" + _ " attachments to accessing only other attachments on the same" + _ " ${terms.bug}. Remember, though, that all those possible domain names " + _ " (such as 1234.your.domain.com) must point to this same" + _ " $terms.Bugzilla instance.", allow_attachment_deletion => "If this option is on, administrators will be able to delete " _ "the content of attachments.", diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl index a48cd2e1de..5606504f92 100644 --- a/template/en/default/attachment/edit.html.tmpl +++ b/template/en/default/attachment/edit.html.tmpl @@ -254,6 +254,29 @@ [% IF !attachment.datasize %] The content of this attachment has been deleted. + [% ELSIF attachment.isurl %] + + + [% IF attachment.datasize < 120 %] + [% attachment.data FILTER html %] + [% ELSE %] + [% attachment.data FILTER truncate(80) FILTER html %] +  ... + [% attachment.data.match(".*(.{20})$").0 FILTER html %] + [% END %] + + + [% ELSIF !Param("allow_attachment_display") %] + +

+ The attachment is not viewable in your browser due to security + restrictions enabled by [% terms.Bugzilla %]. +

+

+ In order to view the attachment, you first have to + download it. +

+ [% ELSIF isviewable %] [% INCLUDE global/textarea.html.tmpl @@ -287,18 +310,6 @@ //--> - [% ELSIF attachment.isurl %] - - - [% IF attachment.datasize < 120 %] - [% attachment.data FILTER html %] - [% ELSE %] - [% attachment.data FILTER truncate(80) FILTER html %] -  ... - [% attachment.data.match(".*(.{20})$").0 FILTER html %] - [% END %] - - [% ELSE %]

diff --git a/template/en/default/attachment/list.html.tmpl b/template/en/default/attachment/list.html.tmpl index 99f51064dc..6849b8ad54 100644 --- a/template/en/default/attachment/list.html.tmpl +++ b/template/en/default/attachment/list.html.tmpl @@ -135,9 +135,11 @@ [% IF attachments.size %] [% IF obsolete_attachments %] - Hide Obsolete ([% obsolete_attachments %]) | + Hide Obsolete ([% obsolete_attachments %]) + [% END %] + [% IF Param("allow_attachment_display") %] + View All [% END %] - View All [% END %] Add an attachment