From: lpsolit%gmail.com <>
Date: Mon, 2 Feb 2009 19:16:02 +0000 (+0000)
Subject: Bug 472206: [SECURITY] Bugzilla should optionally not allow the user to view possibly... 
X-Git-Tag: bugzilla-2.22.7~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=56dbcd21dc5326554a9599a616bba8fbd13728d7;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 <LpSolit@gmail.com> r=mkanat a=LpSolit
---

diff --git a/Bugzilla/Config/Attachment.pm b/Bugzilla/Config/Attachment.pm
index 449908528a..14a001b514 100644
--- a/Bugzilla/Config/Attachment.pm
+++ b/Bugzilla/Config/Attachment.pm
@@ -40,6 +40,12 @@ $Bugzilla::Config::Attachment::sortkey = "025";
 sub get_param_list {
   my $class = shift;
   my @param_list = (
+  {
+   name => 'allow_attachment_display',
+   type => 'b',
+   default => 0
+  },
+
   {
   name => 'allow_attach_url',
   type => 'b',
diff --git a/attachment.cgi b/attachment.cgi
index fe755e8431..a885695472 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -523,8 +523,10 @@ sub view
     $filename =~ s/\\/\\\\/g; # escape backslashes
     $filename =~ s/"/\\"/g; # escape quotes
 
+    my $disposition = Param('allow_attachment_display') ? 'inline' : 'attachment';
+
     print $cgi->header(-type=>"$contenttype; name=\"$filename\"",
-                       -content_disposition=> "inline; filename=\"$filename\"",
+                       -content_disposition=> "$disposition; filename=\"$filename\"",
                        -content_length => $filesize);
 
     if ($thedata) {
diff --git a/template/en/default/admin/params/attachment.html.tmpl b/template/en/default/admin/params/attachment.html.tmpl
index ef89c4af00..c5256cc17e 100644
--- a/template/en/default/admin/params/attachment.html.tmpl
+++ b/template/en/default/admin/params/attachment.html.tmpl
@@ -25,6 +25,15 @@
 %]
 
 [% param_descs = {
+  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." _
+                  "<p>This is a security restriction for installations where " _
+                  "untrusted users may upload attachments that could be potentially " _
+                  "damaging if viewed directly in the browser.</p>",
+
   allow_attach_url => "If this option is on, it will be possible to " _
                       "specify a URL when creating an attachment and " _
                       "treat the URL itself as if it were an attachment.",
diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl
index f34c4593ea..de68b39110 100644
--- a/template/en/default/attachment/edit.html.tmpl
+++ b/template/en/default/attachment/edit.html.tmpl
@@ -262,7 +262,30 @@
         </small>
       </td>
 
-      [% IF isviewable %]
+      [% IF attachment.isurl %]
+        <td width="75%">
+          <a href="[% attachment.data FILTER html %]">
+            [% IF attachment.datasize < 120 %]
+              [% attachment.data FILTER html %]
+            [% ELSE %]
+              [% attachment.data FILTER truncate(80) FILTER html %]
+              &nbsp;...
+              [% attachment.data.match(".*(.{20})$").0 FILTER html %]
+            [% END %]
+          </a>
+        </td>
+      [% ELSIF !Param("allow_attachment_display") %]
+        <td id="view_disabled" width="50%">
+          <p><b>
+            The attachment is not viewable in your browser due to security
+            restrictions enabled by [% terms.Bugzilla %].
+          </b></p>
+          <p><b>
+            In order to view the attachment, you first have to
+            <a href="attachment.cgi?id=[% attachment.id %]">download it</a>.
+          </b></p>
+        </td>
+      [% ELSIF isviewable %]
         <td width="75%">
           <textarea id="editFrame" name="comment" wrap="soft" cols="80"
                     style="height: 400px; width: 100%; display: none;"></textarea>
@@ -287,18 +310,6 @@
             //-->
           </script>
         </td>
-      [% ELSIF attachment.isurl %]
-        <td width="75%">
-          <a href="[% attachment.data FILTER html %]">
-            [% IF attachment.datasize < 120 %]
-              [% attachment.data FILTER html %]
-            [% ELSE %]
-              [% attachment.data FILTER truncate(80) FILTER html %]
-              &nbsp;...
-              [% attachment.data.match(".*(.{20})$").0 FILTER html %]
-            [% END %]
-          </a>
-        </td>
       [% ELSE %]
         <td id="noview" width="50%">
           <p><b>
diff --git a/template/en/default/attachment/list.html.tmpl b/template/en/default/attachment/list.html.tmpl
index 598fbb1b16..601ea74ad9 100644
--- a/template/en/default/attachment/list.html.tmpl
+++ b/template/en/default/attachment/list.html.tmpl
@@ -90,7 +90,7 @@
     <td colspan="[% show_attachment_flags ? 6 : 5 %]">
       <a href="attachment.cgi?bugid=[% bugid %]&amp;action=enter">Create a New Attachment</a> (proposed patch, testcase, etc.)
     </td>
-    [% IF attachments.size %]
+    [% IF attachments.size && Param("allow_attachment_display") %]
       <td>
         <a href="attachment.cgi?bugid=[% bugid %]&amp;action=viewall">View All</a>
       </td>