Bug 450013: (CVE-2010-2757) [SECURITY] Can sudo a user without sending email

r=glob a=LpSolit

4 files changed, 57 insertions, 19 deletions

diff --git a/Bugzilla.pm b/Bugzilla.pm
index 6ecbc27db..d97049678 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -51,10 +51,12 @@ use Bugzilla::Error;
 use Bugzilla::Util;
 use Bugzilla::Field;
 use Bugzilla::Flag;
+use Bugzilla::Token;
 
 use File::Basename;
 use File::Spec::Functions;
 use DateTime::TimeZone;
+use Date::Parse;
 use Safe;
 
 #####################################################################
@@ -341,24 +343,37 @@ sub login {
     # 3: There must be a valid value in the 'sudo' cookie
     # 4: A Bugzilla::User object must exist for the given cookie value
     # 5: That user must NOT be in the 'bz_sudo_protect' group
-    my $sudo_cookie = $class->cgi->cookie('sudo');
-    detaint_natural($sudo_cookie) if defined($sudo_cookie);
-    my $sudo_target;
-    $sudo_target = new Bugzilla::User($sudo_cookie) if defined($sudo_cookie);
-    if (defined($authenticated_user)                 &&
-        $authenticated_user->in_group('bz_sudoers')  &&
-        defined($sudo_cookie)                        &&
-        defined($sudo_target)                        &&
-        !($sudo_target->in_group('bz_sudo_protect'))
-       )
-    {
-        $class->set_user($sudo_target);
-        $class->request_cache->{sudoer} = $authenticated_user;
-        # And make sure that both users have the same Auth object,
-        # since we never call Auth::login for the sudo target.
-        $sudo_target->set_authorizer($authenticated_user->authorizer);
+    my $token = $class->cgi->cookie('sudo');
+    if (defined $authenticated_user && $token) {
+        my ($user_id, $date, $sudo_target_id) = Bugzilla::Token::GetTokenData($token);
+        if (!$user_id
+            || $user_id != $authenticated_user->id
+            || !detaint_natural($sudo_target_id)
+            || (time() - str2time($date) > MAX_SUDO_TOKEN_AGE))
+        {
+            $class->cgi->remove_cookie('sudo');
+            ThrowUserError('sudo_invalid_cookie');
+        }
+
+        my $sudo_target = new Bugzilla::User($sudo_target_id);
+        if ($authenticated_user->in_group('bz_sudoers')
+            && defined $sudo_target
+            && !$sudo_target->in_group('bz_sudo_protect'))
+        {
+            $class->set_user($sudo_target);
+            $class->request_cache->{sudoer} = $authenticated_user;
+            # And make sure that both users have the same Auth object,
+            # since we never call Auth::login for the sudo target.
+            $sudo_target->set_authorizer($authenticated_user->authorizer);
 
-        # NOTE: If you want to do any special logging, do it here.
+            # NOTE: If you want to do any special logging, do it here.
+        }
+        else {
+            delete_token($token);
+            $class->cgi->remove_cookie('sudo');
+            ThrowUserError('sudo_illegal_action', { sudoer => $authenticated_user,
+                                                    target_user => $sudo_target });
+        }
     }
     else {
         $class->set_user($authenticated_user);
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index bfb8d190d..2477d546f 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -160,6 +160,7 @@ use Memoize;
 
     MAX_TOKEN_AGE
     MAX_LOGINCOOKIE_AGE
+    MAX_SUDO_TOKEN_AGE
     MAX_LOGIN_ATTEMPTS
     LOGIN_LOCKOUT_INTERVAL
     MAX_STS_AGE
@@ -415,6 +416,8 @@ use constant TIMETRACKING_FIELDS =>
 use constant MAX_TOKEN_AGE => 3;
 # How many days a logincookie will remain valid if not used.
 use constant MAX_LOGINCOOKIE_AGE => 30;
+# How many seconds (default is 6 hours) a sudo cookie remains valid.
+use constant MAX_SUDO_TOKEN_AGE => 21600;
 
 # Maximum failed logins to lock account for this IP
 use constant MAX_LOGIN_ATTEMPTS => 5;
diff --git a/relogin.cgi b/relogin.cgi
index 7dcbde625..0e04b1bdc 100755
--- a/relogin.cgi
+++ b/relogin.cgi
@@ -147,12 +147,13 @@ elsif ($action eq 'begin-sudo') {
     $reason = substr($reason, $[, 200);
     
     # Calculate the session expiry time (T + 6 hours)
-    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 + MAX_SUDO_TOKEN_AGE, 'GMT');
 
     # For future sessions, store the unique ID of the target user
+    my $token = Bugzilla::Token::_create_token($user->id, 'sudo', $target_user->id);
     $cgi->send_cookie('-name'    => 'sudo',
                       '-expires' => $time_string,
-                      '-value'   => $target_user->id
+                      '-value'   => $token
     );
     
     # For the present, change the values of Bugzilla::user & Bugzilla::sudoer
@@ -173,6 +174,7 @@ elsif ($action eq 'begin-sudo') {
 # end-sudo: End the current sudo session (if one is in progress)
 elsif ($action eq 'end-sudo') {
     # Regardless of our state, delete the sudo cookie if it exists
+    my $token = $cgi->cookie('sudo');
     $cgi->remove_cookie('sudo');
 
     # Are we in an sudo session?
@@ -181,6 +183,8 @@ elsif ($action eq 'end-sudo') {
     if (defined($sudoer)) {
         Bugzilla->sudo_request($sudoer, undef);
     }
+    # Now that the session is over, remove the token from the DB.
+    delete_token($token);
 
     # NOTE: If you want to log the end of an sudo session, so it here.
     
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 635c02971..cdd657050 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1505,6 +1505,22 @@
       [% END %]
     [% END %]
 
+  [% ELSIF error == "sudo_invalid_cookie" %]
+    [% title = "Invalid Sudo Cookie" %]
+    Your sudo cookie is invalid. Either it expired or you didn't start
+    a sudo session correctly. Refresh the page or load another page
+    to continue what you are doing as yourself.
+
+  [% ELSIF error == "sudo_illegal_action" %]
+    [% title = "Impersonation Not Authorized" %]
+    [% IF NOT sudoer.in_group("bz_sudoers") %]
+      You are not allowed to impersonate users.
+    [% ELSIF target_user AND target_user.in_group("bz_sudo_protect") %]
+      You are not allowed to impersonate [% target_user.identity FILTER html %].
+    [% ELSE %]
+      The user you tried to impersonate doesn't exist.
+    [% END %]
+
   [% ELSIF error == "sudo_in_progress" %]
     [% title = "Session In Progress" %]
     A sudo session (impersonating [% target FILTER html %]) is in progress.