Commit 3b2f0ca8 authored by karl%kornel.name's avatar karl%kornel.name

Bug 312441: relogin.cgi allows you to impersonate user accounts you are not…

Bug 312441: relogin.cgi allows you to impersonate user accounts you are not allowed to see when 'usevisibilitygroups' is on - Patch by A. Karl Kornel <karl@kornel.name> r=LpSolit a=justdave
parent 5ad7900f
...@@ -31,6 +31,7 @@ use Bugzilla; ...@@ -31,6 +31,7 @@ use Bugzilla;
use Bugzilla::BugMail; use Bugzilla::BugMail;
use Bugzilla::Constants; use Bugzilla::Constants;
use Bugzilla::Error; use Bugzilla::Error;
use Bugzilla::Token;
use Bugzilla::User; use Bugzilla::User;
use Bugzilla::Util; use Bugzilla::Util;
use Date::Format; use Date::Format;
...@@ -43,8 +44,8 @@ my $action = $cgi->param('action') || 'logout'; ...@@ -43,8 +44,8 @@ my $action = $cgi->param('action') || 'logout';
my $vars = {}; my $vars = {};
my $target; my $target;
# sudo: Display the sudo information & login page # prepare-sudo: Display the sudo information & login page
if ($action eq 'sudo') { if ($action eq 'prepare-sudo') {
# We must have a logged-in user to do this # We must have a logged-in user to do this
# That user must be in the 'bz_sudoers' group # That user must be in the 'bz_sudoers' group
my $user = Bugzilla->login(LOGIN_REQUIRED); my $user = Bugzilla->login(LOGIN_REQUIRED);
...@@ -60,77 +61,44 @@ if ($action eq 'sudo') { ...@@ -60,77 +61,44 @@ if ($action eq 'sudo') {
ThrowUserError('sudo_in_progress', { target => $user->login }); ThrowUserError('sudo_in_progress', { target => $user->login });
} }
# We may have been given a value to put into the field # Keep a temporary record of the user visitng this page
# Don't pass it through unless it's actually a user $vars->{'token'} = Bugzilla::Token::IssueSessionToken('sudo_prepared');
# Could the default value be protected? Maybe, but we will save the
# disappointment for later!
if (defined($cgi->param('target_login')) &&
Bugzilla::User::login_to_id($cgi->param('target_login')) != 0)
{
$vars->{'target_login_default'} = $cgi->param('target_login');
}
# Show the sudo page # Show the sudo page
$vars->{'will_logout'} = $user->get_flag('can_logout'); $vars->{'target_login_default'} = $cgi->param('target_login');
$vars->{'reason_default'} = $cgi->param('reason');
$target = 'admin/sudo.html.tmpl'; $target = 'admin/sudo.html.tmpl';
} }
# transition-sudo: Validate target, logout user, and redirect for session start # begin-sudo: Confirm login and start sudo session
elsif ($action eq 'sudo-transition') { elsif ($action eq 'begin-sudo') {
# Get the current user # We must be sure that the user is authenticating by providing a login
my $user = Bugzilla->login(LOGIN_REQUIRED); # and password.
unless ($user->in_group('bz_sudoers')) { # We only need to do this for authentication methods that involve Bugzilla
ThrowUserError('auth_failure', { group => 'bz_sudoers', # directly obtaining a login (i.e. normal CGI login), as opposed to other
action => 'begin', # methods (like Environment vars login). We assume that if a user can log
object => 'sudo_session' } # out, they can also log in:
);
} # First, record if Bugzilla_login and Bugzilla_password were provided
my $credentials_provided;
# Get & verify the target user (the user who we will be impersonating) if (defined($cgi->param('Bugzilla_login'))
unless (defined($cgi->param('target_login')) && && defined($cgi->param('Bugzilla_password')))
Bugzilla::User::login_to_id($cgi->param('target_login')) != 0)
{
ThrowUserError('invalid_username',
{ 'name' => $cgi->param('target_login') }
);
}
my $target_user = new Bugzilla::User(
Bugzilla::User::login_to_id($cgi->param('target_login'))
);
unless (defined($target_user) &&
$target_user->id != 0)
{
ThrowUserError('invalid_username',
{ 'name' => $cgi->param('target_login') }
);
}
unless (defined($cgi->param('target_login')) &&
$target_user->id != 0)
{ {
ThrowUserError('invalid_username', $credentials_provided = 1;
{ 'name' => $cgi->param('target_login') }
);
} }
if ($target_user->in_group('bz_sudo_protect')) {
ThrowUserError('sudo_protected', { login => $target_user->login });
}
# If we have a reason passed in, keep it under 200 characters
my $reason = $cgi->param('reason') || '';
$reason = substr($reason, $[, 200);
my $reason_string = '&reason=' . url_quote($reason);
# Log out and redirect user to the new page # Next, log in the user
Bugzilla->logout();
$target = 'relogin.cgi';
print $cgi->redirect($target . '?action=begin-sudo&target_login=' .
url_quote($target_user->login) . $reason_string);
exit;
}
# begin-sudo: Confirm login and start sudo session
elsif ($action eq 'begin-sudo') {
# We must have a logged-in user to do this
# That user must be in the 'bz_sudoers' group
my $user = Bugzilla->login(LOGIN_REQUIRED); my $user = Bugzilla->login(LOGIN_REQUIRED);
# At this point, the user is logged in. However, if they used a method
# where they could have provided a username/password (i.e. CGI), but they
# did not provide a username/password, then throw an error.
if ($user->get_flag('can_logout') && !$credentials_provided) {
ThrowUserError('sudo_password_required',
{ target_login => $cgi->param('target_login'),
reason => $cgi->param('reason')});
}
# The user must be in the 'bz_sudoers' group
unless ($user->in_group('bz_sudoers')) { unless ($user->in_group('bz_sudoers')) {
ThrowUserError('auth_failure', { group => 'bz_sudoers', ThrowUserError('auth_failure', { group => 'bz_sudoers',
action => 'begin', action => 'begin',
...@@ -138,35 +106,41 @@ elsif ($action eq 'begin-sudo') { ...@@ -138,35 +106,41 @@ elsif ($action eq 'begin-sudo') {
); );
} }
# Get & verify the target user (the user who we will be impersonating) # Do not try to start a new session if one is already in progress!
unless (defined($cgi->param('target_login')) && if (defined(Bugzilla->sudoer)) {
Bugzilla::User::login_to_id($cgi->param('target_login')) != 0) ThrowUserError('sudo_in_progress', { target => $user->login });
{
ThrowUserError('invalid_username',
{ 'name' => $cgi->param('target_login') }
);
} }
my $target_user = new Bugzilla::User(
Bugzilla::User::login_to_id($cgi->param('target_login')) # Did the user actually go trough the 'sudo-prepare' action? Do some
); # checks on the token the action should have left.
unless (defined($target_user) && my ($token_user, $token_timestamp, $token_data) =
$target_user->id != 0) Bugzilla::Token::GetTokenData($cgi->param('token'));
unless (defined($token_user)
&& defined($token_data)
&& ($token_user == $user->id)
&& ($token_data eq 'sudo_prepared'))
{ {
ThrowUserError('invalid_username', ThrowUserError('sudo_preparation_required',
{ 'name' => $cgi->param('target_login') } { target_login => $cgi->param('target_login'),
); reason => $cgi->param('reason')});
} }
unless (defined($cgi->param('target_login')) && Bugzilla::Token::DeleteToken($cgi->param('token'));
$target_user->id != 0)
# Get & verify the target user (the user who we will be impersonating)
my $target_user =
Bugzilla::User->new_from_login($cgi->param('target_login'));
unless (defined($target_user)
&& $target_user->id
&& $user->can_see_user($target_user))
{ {
ThrowUserError('invalid_username', ThrowUserError('user_match_failed',
{ 'name' => $cgi->param('target_login') } { 'name' => $cgi->param('target_login') }
); );
} }
if ($target_user->in_group('bz_sudo_protect')) { if ($target_user->in_group('bz_sudo_protect')) {
ThrowUserError('sudo_protected', { login => $target_user->login }); ThrowUserError('sudo_protected', { login => $target_user->login });
} }
# If we have a reason passed in, keep it under 200 characters # If we have a reason passed in, keep it under 200 characters
my $reason = $cgi->param('reason') || ''; my $reason = $cgi->param('reason') || '';
$reason = substr($reason, $[, 200); $reason = substr($reason, $[, 200);
...@@ -175,13 +149,13 @@ elsif ($action eq 'begin-sudo') { ...@@ -175,13 +149,13 @@ elsif ($action eq 'begin-sudo') {
my $time_string = time2str('%a, %d-%b-%Y %T %Z', time+(6*60*60), 'GMT'); my $time_string = time2str('%a, %d-%b-%Y %T %Z', time+(6*60*60), 'GMT');
# For future sessions, store the unique ID of the target user # For future sessions, store the unique ID of the target user
Bugzilla->cgi->send_cookie('-name' => 'sudo', $cgi->send_cookie('-name' => 'sudo',
'-expires' => $time_string, '-expires' => $time_string,
'-value' => $target_user->id '-value' => $target_user->id
); );
# For the present, change the values of Bugzilla::user & Bugzilla::sudoer # For the present, change the values of Bugzilla::user & Bugzilla::sudoer
Bugzilla->sudo_request($target_user, Bugzilla->user); Bugzilla->sudo_request($target_user, $user);
# NOTE: If you want to log the start of an sudo session, do it here. # NOTE: If you want to log the start of an sudo session, do it here.
...@@ -205,7 +179,6 @@ elsif ($action eq 'end-sudo') { ...@@ -205,7 +179,6 @@ elsif ($action eq 'end-sudo') {
Bugzilla->login(LOGIN_OPTIONAL); Bugzilla->login(LOGIN_OPTIONAL);
my $sudoer = Bugzilla->sudoer; my $sudoer = Bugzilla->sudoer;
if (defined($sudoer)) { if (defined($sudoer)) {
Bugzilla->logout_request();
Bugzilla->sudo_request($sudoer, undef); Bugzilla->sudo_request($sudoer, undef);
} }
...@@ -225,10 +198,6 @@ elsif ($action eq 'logout') { ...@@ -225,10 +198,6 @@ elsif ($action eq 'logout') {
Bugzilla->logout(); Bugzilla->logout();
my $template = Bugzilla->template;
my $cgi = Bugzilla->cgi;
print $cgi->header();
$vars->{'message'} = "logged_out"; $vars->{'message'} = "logged_out";
$target = 'global/message.html.tmpl'; $target = 'global/message.html.tmpl';
} }
......
...@@ -74,7 +74,7 @@ ...@@ -74,7 +74,7 @@
[% IF user.groups.bz_sudoers %] [% IF user.groups.bz_sudoers %]
<br> <br>
You are a member of the <b>bz_sudoers</b> group, so you can You are a member of the <b>bz_sudoers</b> group, so you can
<a href="relogin.cgi?action=sudo">impersonate someone else</a>. <a href="relogin.cgi?action=prepare-sudo">impersonate someone else</a>.
[% END %] [% END %]
</td> </td>
</tr> </tr>
......
...@@ -66,7 +66,8 @@ ...@@ -66,7 +66,8 @@
<p> <p>
Next, please take a moment to explain why you are doing this:<br> Next, please take a moment to explain why you are doing this:<br>
<input type="text" name="reason" size="80" maxlength="200"> <input type="text" name="reason" size="80" maxlength="200" value="
[%- reason_default FILTER html %]">
</p> </p>
<p> <p>
...@@ -75,21 +76,27 @@ ...@@ -75,21 +76,27 @@
are impersonating them. are impersonating them.
</p> </p>
<p> [% IF user.get_flag("can_logout") %]
Finally, click the button to begin the session:
<input type="submit" value="Begin Session">
<input type="hidden" name="action" value="sudo-transition">
</p>
[% IF will_logout %]
<p> <p>
When you press the button, you may be logged out and asked to log in Finally, enter your [% terms.Bugzilla %] password:
again. This is done for two reasons. First of all, it is done to reduce <input type="hidden" name="Bugzilla_login" value="
[%- user.login FILTER html %]">
<input type="password" name="Bugzilla_password" maxlength="20" size="20">
<br>
This is done for two reasons. First of all, it is done to reduce
the chances of someone doing large amounts of damage using your the chances of someone doing large amounts of damage using your
already-logged-in account. Second, it is there to force you to take the already-logged-in account. Second, it is there to force you to take the
time to consider if you really need to use this feature. time to consider if you really need to use this feature.
</p> </p>
[% END %] [% END %]
<p>
Click the button to begin the session:
<input type="submit" value="Begin Session">
<input type="hidden" name="action" value="begin-sudo">
<input type="hidden" name="token" value="[% token FILTER html %]">
</p>
</form> </form>
[% PROCESS global/footer.html.tmpl %] [% PROCESS global/footer.html.tmpl %]
...@@ -32,7 +32,7 @@ ...@@ -32,7 +32,7 @@
value="[% otheruser.login FILTER html %]" /> value="[% otheruser.login FILTER html %]" />
[% IF !otheruser.groups.bz_sudo_protect %] [% IF !otheruser.groups.bz_sudo_protect %]
<br /> <br />
<a href="relogin.cgi?action=sudo&amp;target_login= <a href="relogin.cgi?action=prepare-sudo&amp;target_login=
[%- otheruser.login FILTER html %]">Impersonate this user</a> [%- otheruser.login FILTER html %]">Impersonate this user</a>
[% END %] [% END %]
[% END %] [% END %]
......
...@@ -1128,6 +1128,20 @@ ...@@ -1128,6 +1128,20 @@
An sudo session (impersonating [% target FILTER html %]) is in progress. An sudo session (impersonating [% target FILTER html %]) is in progress.
End that session (using the link in the footer) before starting a new one. End that session (using the link in the footer) before starting a new one.
[% ELSIF error == "sudo_password_required" %]
[% title = "Password Required" %]
Your [% terms.Bugzilla %] password is required to begin a sudo
session. Please <a href="relogin.cgi?action=prepare-sudo&target_login=
[%- target_login FILTER html %]&reason=
[%- reason FILTER html %]">go back</a> and enter your password</a>.
[% ELSIF error == "sudo_preparation_required" %]
[% title = "Preparation Required" %]
You may not start a sudo session directly. Please
<a href="relogin.cgi?action=prepare-sudo&target_login=
[%- target_login FILTER html %]&reason=
[%- reason FILTER html %]">start your session normally</a>.
[% ELSIF error == "sudo_protected" %] [% ELSIF error == "sudo_protected" %]
[% title = "User Protected" %] [% title = "User Protected" %]
The user [% login FILTER html %] may not be impersonated by sudoers. The user [% login FILTER html %] may not be impersonated by sudoers.
...@@ -1202,6 +1216,11 @@ ...@@ -1202,6 +1216,11 @@
[% title = "Login Name Required" %] [% title = "Login Name Required" %]
You must enter a login name for the new user. You must enter a login name for the new user.
[% ELSIF error == "user_match_failed" %]
[% title = "Match Failed" %]
<tt>[% name FILTER html %]</tt> does not exist or you are not allowed
to see that user.
[% ELSIF error == "votes_must_be_nonnegative" %] [% ELSIF error == "votes_must_be_nonnegative" %]
[% title = "Votes Must Be Non-negative" %] [% title = "Votes Must Be Non-negative" %]
Only use non-negative numbers for your [% terms.bug %] votes. Only use non-negative numbers for your [% terms.bug %] votes.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment