Blob Blame History Raw
=== modified file 'Bugzilla/Attachment/PatchReader.pm'
--- Bugzilla/Attachment/PatchReader.pm	2008-06-29 22:35:28 +0000
+++ Bugzilla/Attachment/PatchReader.pm	2011-07-07 06:04:15 +0000
@@ -37,6 +37,7 @@
         $last_reader->sends_data_to(new PatchReader::DiffPrinter::raw());
         # Actually print out the patch.
         print $cgi->header(-type => 'text/plain',
+                           -x_content_type_options => "nosniff",
                            -expires => '+3M');
         disable_utf8();
         $reader->iterate_string('Attachment ' . $attachment->id, $attachment->data);
@@ -118,6 +119,7 @@
         $last_reader->sends_data_to(new PatchReader::DiffPrinter::raw());
         # Actually print out the patch.
         print $cgi->header(-type => 'text/plain',
+                           -x_content_type_options => "nosniff",
                            -expires => '+3M');
         disable_utf8();
     }

=== modified file 'attachment.cgi'
--- attachment.cgi	2009-09-30 08:53:25 +0000
+++ attachment.cgi	2011-07-21 06:21:26 +0000
@@ -71,10 +71,13 @@
 
 # Determine whether to use the action specified by the user or the default.
 my $action = $cgi->param('action') || 'view';
+my $format = $cgi->param('format') || '';
 
 # You must use the appropriate urlbase/sslbase param when doing anything
-# but viewing an attachment.
-if ($action ne 'view') {
+# but viewing an attachment, or a raw diff.
+if ($action ne 'view'
+    && (($action !~ /^(?:interdiff|diff)$/) || $format ne 'raw'))
+{
     my $urlbase = Bugzilla->params->{'urlbase'};
     my $sslbase = Bugzilla->params->{'sslbase'};
     my $path_regexp = $sslbase ? qr/^(\Q$urlbase\E|\Q$sslbase\E)/ : qr/^\Q$urlbase\E/;
@@ -172,7 +175,8 @@
     # non-natural, so use the original value from $cgi in our exception
     # message here.
     detaint_natural($attach_id)
-     || ThrowUserError("invalid_attach_id", { attach_id => $cgi->param($param) });
+      || ThrowUserError("invalid_attach_id",
+                        { attach_id => scalar $cgi->param($param) });
   
     # Make sure the attachment exists in the database.
     my $attachment = Bugzilla::Attachment->get($attach_id)
@@ -249,53 +253,71 @@
                         { bug_id => $bugid });
 }
 
-################################################################################
-# Functions
-################################################################################
+# Gets the attachment object(s) generated by validateID, while ensuring
+# attachbase and token authentication is used when required.
+sub get_attachment {
+    my @field_names = @_ ? @_ : qw(id);
 
-# Display an attachment.
-sub view {
-    my $attachment;
+    my %attachments;
 
     if (use_attachbase()) {
-        $attachment = validateID(undef, 1);
-        # Replace %bugid% by the ID of the bug the attachment belongs to, if present.
+        # Load each attachment, and ensure they are all from the same bug
+        my $bug_id = 0;
+        foreach my $field_name (@field_names) {
+            my $attachment = validateID($field_name, 1);
+            if (!$bug_id) {
+                $bug_id = $attachment->bug_id;
+            } elsif ($attachment->bug_id != $bug_id) {
+                ThrowUserError('attachment_bug_id_mismatch');
+            }
+            $attachments{$field_name} = $attachment;
+        }
         my $attachbase = Bugzilla->params->{'attachment_base'};
-        my $bug_id = $attachment->bug_id;
         $attachbase =~ s/%bugid%/$bug_id/;
-        my $path = 'attachment.cgi?id=' . $attachment->id;
-        # The user is allowed to override the content type of the attachment.
-        if (defined $cgi->param('content_type')) {
-            $path .= '&content_type=' . url_quote($cgi->param('content_type'));
-        }
+        my @args = map { $_ . '=' . $attachments{$_}->id } @field_names;
+        my $cgi_params = $cgi->canonicalise_query(@field_names, 't',
+            'Bugzilla_login', 'Bugzilla_password');
+        push(@args, $cgi_params) if $cgi_params;
+        my $path = 'attachment.cgi?' . join('&', @args);
 
         # Make sure the attachment is served from the correct server.
         if ($cgi->self_url !~ /^\Q$attachbase\E/) {
             # We couldn't call Bugzilla->login earlier as we first had to make sure
             # we were not going to request credentials on the alternate host.
             Bugzilla->login();
-            if (attachmentIsPublic($attachment)) {
+            if (all_attachments_are_public(\%attachments)) {
                 # No need for a token; redirect to attachment base.
                 print $cgi->redirect(-location => $attachbase . $path);
                 exit;
             } else {
                 # Make sure the user can view the attachment.
-                check_can_access($attachment);
+                foreach my $field_name (@field_names) {
+                    check_can_access($attachments{$field_name});
+                }
                 # Create a token and redirect.
-                my $token = url_quote(issue_session_token($attachment->id));
+                my $token = url_quote(issue_session_token(pack_token_data(\%attachments)));
                 print $cgi->redirect(-location => $attachbase . "$path&t=$token");
                 exit;
             }
         } else {
             # No need to validate the token for public attachments. We cannot request
             # credentials as we are on the alternate host.
-            if (!attachmentIsPublic($attachment)) {
+            if (!all_attachments_are_public(\%attachments)) {
                 my $token = $cgi->param('t');
-                my ($userid, undef, $token_attach_id) = Bugzilla::Token::GetTokenData($token);
-                unless ($userid
-                        && detaint_natural($token_attach_id)
-                        && ($token_attach_id == $attachment->id))
-                {
+                my ($userid, undef, $token_data) = Bugzilla::Token::GetTokenData($token);
+                my %token_data = unpack_token_data($token_data);
+                my $valid_token = 1;
+                foreach my $field_name (@field_names) {
+                    my $token_id = $token_data{$field_name};
+                    if (!$token_id
+                        || !detaint_natural($token_id)
+                        || $attachments{$field_name}->id != $token_id)
+                    {
+                        $valid_token = 0;
+                        last;
+                    }
+                }
+                unless ($userid && $valid_token) {
                     # Not a valid token.
                     print $cgi->redirect('-location' => correct_urlbase() . $path);
                     exit;
@@ -309,8 +331,48 @@
     } else {
         # No alternate host is used. Request credentials if required.
         Bugzilla->login();
-        $attachment = validateID();
-    }
+        foreach my $field_name (@field_names) {
+            $attachments{$field_name} = validateID($field_name);
+        }
+    }
+
+    return wantarray
+        ? map { $attachments{$_} } @field_names
+        : $attachments{$field_names[0]};
+}
+
+sub all_attachments_are_public {
+    my $attachments = shift;
+    foreach my $field_name (keys %$attachments) {
+        if (!attachmentIsPublic($attachments->{$field_name})) {
+            return 0;
+        }
+    }
+    return 1;
+}
+
+sub pack_token_data {
+    my $attachments = shift;
+    return join(' ', map { $_ . '=' . $attachments->{$_}->id } keys %$attachments);
+}
+
+sub unpack_token_data {
+    my @token_data = split(/ /, shift || '');
+    my %data;
+    foreach my $token (@token_data) {
+        my ($field_name, $attach_id) = split('=', $token);
+        $data{$field_name} = $attach_id;
+    }
+    return %data;
+}
+
+################################################################################
+# Functions
+################################################################################
+
+# Display an attachment.
+sub view {
+    my $attachment = get_attachment();
 
     # At this point, Bugzilla->login has been called if it had to.
     my $contenttype = $attachment->contenttype;
@@ -345,9 +407,14 @@
 
 sub interdiff {
     # Retrieve and validate parameters
-    my $old_attachment = validateID('oldid');
-    my $new_attachment = validateID('newid');
     my $format = validateFormat('html', 'raw');
+    my($old_attachment, $new_attachment);
+    if ($format eq 'raw') {
+        ($old_attachment, $new_attachment) = get_attachment('oldid', 'newid');
+    } else {
+        $old_attachment = validateID('oldid');
+        $new_attachment = validateID('newid');
+    }
     my $context = validateContext();
 
     Bugzilla::Attachment::PatchReader::process_interdiff(
@@ -356,8 +423,8 @@
 
 sub diff {
     # Retrieve and validate parameters
-    my $attachment = validateID();
     my $format = validateFormat('html', 'raw');
+    my $attachment = $format eq 'raw' ? get_attachment() : validateID();
     my $context = validateContext();
 
     # If it is not a patch, view normally.

=== modified file 'contrib/fixperms.pl' (properties changed: -x to +x)
=== modified file 'template/en/default/global/user-error.html.tmpl'
--- template/en/default/global/user-error.html.tmpl	2010-11-02 23:12:13 +0000
+++ template/en/default/global/user-error.html.tmpl	2011-07-07 06:04:15 +0000
@@ -102,6 +102,11 @@
     [% terms.Bug %] aliases cannot be longer than 20 characters.
     Please choose a shorter alias.
 
+  [% ELSIF error == "attachment_bug_id_mismatch" %]
+    [% title = "Invalid Attachments" %]
+    You tried to perform an action on attachments from different [% terms.bugs %].
+    This operation requires all attachments to be from the same [% terms.bug %].
+
   [% ELSIF error == "auth_cant_create_account" %]
     [% title = "Can't create accounts" %]
     This site is using an authentication scheme which does not permit