=== 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