From 8ae3947f54cf1ee266b2f6145bd6320ecaeb9948 Mon Sep 17 00:00:00 2001 From: "lpsolit%gmail.com" <> Date: Wed, 30 Dec 2009 14:54:28 +0000 Subject: Bug 532518: Credentials are not checked correctly when viewing one attachment from another bug's alternate host - Patch by Frédéric Buclin r=mkanat a=LpSolit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- attachment.cgi | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) (limited to 'attachment.cgi') diff --git a/attachment.cgi b/attachment.cgi index 20a96d09d..0181f8cad 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -249,8 +249,34 @@ sub view { # Make sure the attachment is served from the correct server. my $bug_id = $attachment->bug_id; - if (!$cgi->url_is_attachment_base($bug_id)) { - # We couldn't call Bugzilla->login earlier as we first had to + if ($cgi->url_is_attachment_base($bug_id)) { + # No need to validate the token for public attachments. We cannot request + # credentials as we are on the alternate host. + if (!attachmentIsPublic($attachment)) { + 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)) + { + # Not a valid token. + print $cgi->redirect('-location' => correct_urlbase() . $path); + exit; + } + # Change current user without creating cookies. + Bugzilla->set_user(new Bugzilla::User($userid)); + # Tokens are single use only, delete it. + delete_token($token); + } + } + elsif ($cgi->url_is_attachment_base) { + # If we come here, this means that each bug has its own host + # for attachments, and that we are trying to view one attachment + # using another bug's host. That's not desired. + $cgi->redirect_to_urlbase; + } + else { + # 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(); @@ -270,25 +296,6 @@ sub view { 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)) { - 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)) - { - # Not a valid token. - print $cgi->redirect('-location' => correct_urlbase() . $path); - exit; - } - # Change current user without creating cookies. - Bugzilla->set_user(new Bugzilla::User($userid)); - # Tokens are single use only, delete it. - delete_token($token); - } } } else { do_ssl_redirect_if_required(); -- cgit v1.2.3-24-g4f1b