Last Comment Bug 450013 - (CVE-2010-2757) [SECURITY] Can sudo a user without sending email
(CVE-2010-2757)
: [SECURITY] Can sudo a user without sending email
Status: RESOLVED FIXED
[infrasec:access]
:
Product: Bugzilla
Classification: Server Software
Component: User Accounts (show other bugs)
: 2.22
: All All
: -- normal (vote)
: Bugzilla 3.2
Assigned To: Frédéric Buclin
: default-qa
Mentors:
: 321015 556649 (view as bug list)
Depends on:
Blocks: q2-review-bmo 580214
  Show dependency treegraph
 
Reported: 2008-08-10 05:29 PDT by Bradley Baetz (:bbaetz)
Modified: 2010-12-23 14:42 PST (History)
7 users (show)
LpSolit: approval+
LpSolit: approval4.0+
LpSolit: blocking4.0+
LpSolit: approval3.6+
LpSolit: blocking3.6.2+
LpSolit: approval3.4+
LpSolit: blocking3.4.8+
LpSolit: approval3.2+
LpSolit: blocking3.2.8+
mkanat: blocking3.2-
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (6.94 KB, patch)
2010-05-13 17:15 PDT, Frédéric Buclin
mkanat: review-
Details | Diff | Review
patch for 3.6 - 4.2, v2 (7.04 KB, patch)
2010-07-05 04:56 PDT, Frédéric Buclin
glob: review+
Details | Diff | Review
patch for 3.4, v1 (7.08 KB, patch)
2010-07-18 17:16 PDT, Frédéric Buclin
glob: review+
Details | Diff | Review
patch for 3.2, v1 (7.04 KB, patch)
2010-07-18 17:24 PDT, Frédéric Buclin
glob: review+
Details | Diff | Review

Description Bradley Baetz (:bbaetz) 2008-08-10 05:29:46 PDT
The cookie for sudo is the userid.

1. Find out user's id (some queries with debug=1 will expose this)
2. Add cookie 'sudo=<id>'
3. Visit bugzilla site

Actual:

Have sudoed user

Expected:

Fails.

This needs to use a cookie from the token table (that verifies the (originaluser, targetuser) tuple, or something similar. You need to have privileges to sudo, so its not a permissions exploit, but its still not good.....
Comment 1 Frédéric Buclin 2008-08-10 09:31:13 PDT
I wouldn't qualify this bug as critical as you cannot do more than what the normal workflow lets you do. The only difference is that no email is sent to the one being sudo'ed, which is by far much less critical than being able to sudo someone in the bz_sudo_protect group (which you cannot do) or being able to sudo someone despite you don't belong to the bz_sudoers group (which you cannot do).
Comment 2 Max Kanat-Alexander 2008-08-10 11:42:38 PDT
Heck, I didn't even want the email to be sent, when we implemented sudo. :-) So I'm not super-concerned about this, but I do agree it's a security bug in a minor sense.
Comment 3 Max Kanat-Alexander 2008-08-10 11:43:18 PDT
We're too close to 3.2 and this is too minor to be a blocker.
Comment 4 Reed Loden [:reed] (use needinfo?) 2010-04-01 17:23:57 PDT
*** Bug 556649 has been marked as a duplicate of this bug. ***
Comment 5 Frédéric Buclin 2010-04-09 09:38:30 PDT
I will give it a look once 3.6 is released.
Comment 6 Frédéric Buclin 2010-05-13 17:15:35 PDT
Created attachment 445238 [details] [diff] [review]
patch, v1

The cookie now contains a token, which is only created when using the correct way to impersonate users. If something goes wrong, we now throw an error rather than silently falling back to the sudoer (impersonation is critical enough to notify the sudoer).
Comment 7 Max Kanat-Alexander 2010-05-14 08:01:57 PDT
Comment on attachment 445238 [details] [diff] [review]
patch, v1

  Man, we really need to make tokens into objects. That's for a later time, though.


>+        if (!$user_id
>+            || $user_id != $authenticated_user->id
>+            || !detaint_natural($sudo_target_id)
>+            || time() - str2time($date) > MAX_SUDO_TOKEN_AGE)

  I'd like to see parens around that last time()- condition, to make the precedence clearer.

>Index: relogin.cgi
>+    my $time_string = time2str('%a, %d-%b-%Y %T %Z', time+(MAX_SUDO_TOKEN_AGE), 

  Those parens probably aren't necessary now.

>Index: template/en/default/global/user-error.html.tmpl
>+  [% 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.

  For the case where it expired, we should tell the user that they can just refresh the page or load another page to continue what they are doing as themselves.

>+  [% ELSIF error == "sudo_illegal_action" %]
>+    [% ELSE %]
>+      The user you try to impersonate doesn't exist.

  s/try/tried/
Comment 8 Frédéric Buclin 2010-07-05 04:56:57 PDT
Created attachment 456033 [details] [diff] [review]
patch for 3.6 - 4.2, v2
Comment 9 Byron Jones ‹:glob› 2010-07-18 04:03:47 PDT
Comment on attachment 456033 [details] [diff] [review]
patch for 3.6 - 4.2, v2

r=glob
Comment 10 Frédéric Buclin 2010-07-18 17:03:10 PDT
It needs a backport for 3.4 and 3.2.
Comment 11 Frédéric Buclin 2010-07-18 17:16:31 PDT
Created attachment 458238 [details] [diff] [review]
patch for 3.4, v1

Same patch as for 3.6-4.2, except that it fixes a small bitrot due to context lines which changed in Constants.pm.
Comment 12 Frédéric Buclin 2010-07-18 17:24:52 PDT
Created attachment 458240 [details] [diff] [review]
patch for 3.2, v1

Minor bitrot in Bugzilla.pm for 3.2 due to |use DateTime::TimeZone| which doesn't exist there. No other changes.
Comment 13 Byron Jones ‹:glob› 2010-07-20 01:05:14 PDT
Comment on attachment 458238 [details] [diff] [review]
patch for 3.4, v1

r=glob
Comment 14 Byron Jones ‹:glob› 2010-07-20 01:05:56 PDT
Comment on attachment 458240 [details] [diff] [review]
patch for 3.2, v1

r=glob
Comment 15 Frédéric Buclin 2010-07-20 01:08:08 PDT
ok, this bug is ready for checkin. Thanks glob for the reviews.
Comment 16 Frédéric Buclin 2010-08-04 14:48:22 PDT
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla.pm
modified relogin.cgi
modified Bugzilla/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7429.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla.pm
modified relogin.cgi
modified Bugzilla/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7370.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.6/
modified Bugzilla.pm
modified relogin.cgi
modified Bugzilla/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 7158.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.4/
modified Bugzilla.pm
modified relogin.cgi
modified Bugzilla/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 6772.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/3.2/
modified Bugzilla.pm
modified relogin.cgi
modified Bugzilla/Constants.pm
modified template/en/default/global/user-error.html.tmpl
Committed revision 6393.
Comment 17 Max Kanat-Alexander 2010-08-05 21:39:50 PDT
Security advisory sent, unlocking bug.
Comment 18 Reed Loden [:reed] (use needinfo?) 2010-12-23 14:42:09 PST
*** Bug 321015 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.