Last Comment Bug 395632 - [SECURITY] XML-RPC WebService Bugzilla::User::offer_account_by_email does not check createemailregexp
: [SECURITY] XML-RPC WebService Bugzilla::User::offer_account_by_email does not...
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: WebService (show other bugs)
: 3.0.1
: All All
: -- critical (vote)
: Bugzilla 3.0
Assigned To: Max Kanat-Alexander
: default-qa
Mentors:
Depends on:
Blocks: 359532 395715
  Show dependency treegraph
 
Reported: 2007-09-10 00:03 PDT by Sascha Jensen
Modified: 2012-03-21 07:33 PDT (History)
3 users (show)
mkanat: approval+
mkanat: blocking3.1.2+
mkanat: approval3.0+
mkanat: blocking3.0.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (2.04 KB, patch)
2007-09-10 00:13 PDT, Max Kanat-Alexander
wurblzap: review+
LpSolit: review+
Details | Diff | Review
v1, 3.0 (1.38 KB, patch)
2007-09-10 14:16 PDT, Max Kanat-Alexander
LpSolit: review+
Details | Diff | Review

Description Sascha Jensen 2007-09-10 00:03:00 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.6) Gecko/20070725 Firefox/2.0.0.6
Build Identifier: 3.0.1

Despite leaving createemailregexp parameter blank (disable account creation by email) it is possible to use Bugzilla::User::offer_account_by_email. Any other kind of reular expression in createemailregexp is ignored, too.
It seems the value of createemailregexp is not checked.

Reproducible: Always

Steps to Reproduce:
1. set createemailregexp to whatever you like
2. fill appropriate values into the folowing python script

import xmlrpclib
server_proxy = xmlrpclib.ServerProxy( URL_TO_XMLRPCCGI ) 
server_proxy.User.offer_account_by_email( {'email':ANYMAILADDRESS} )

3. run the script



I already posted this issue on a newsgroup:
news://news.mozilla.org:119/TpqdnR2hvJwq1HzbnZ2dnUVZ_qKgnZ2d@mozilla.org
Comment 1 Max Kanat-Alexander 2007-09-10 00:13:53 PDT
Created attachment 280316 [details] [diff] [review]
v1

Yeah, that's my fault, and this is serious enough that we should release ASAP.
Comment 2 Max Kanat-Alexander 2007-09-10 00:14:23 PDT
This compromises requirelogin installations.
Comment 3 Marc Schumann [:Wurblzap] 2007-09-10 03:24:38 PDT
Comment on attachment 280316 [details] [diff] [review]
v1

Tested; works as expected. r=Wurblzap.

I wonder whether we should rather move these checks from Bugzilla::WebService::User and createaccount.cgi both into Bugzilla::User::check_login_name_for_creation.
Comment 4 Frédéric Buclin 2007-09-10 04:22:01 PDT
(In reply to comment #3)
> I wonder whether we should rather move these checks from
> Bugzilla::WebService::User and createaccount.cgi both into
> Bugzilla::User::check_login_name_for_creation.

No, because an admin is free to create an account which doesn't satisfy createemailregexp. All the admin needs to know is whether the login name is already in use or not.
Comment 5 Frédéric Buclin 2007-09-10 04:47:10 PDT
Comment on attachment 280316 [details] [diff] [review]
v1

>+    elsif ($email !~ /$createexp/) {
>+        ThrowUserError("account_creation_restricted");
>+    }

r=LpSolit for tip, but it needs a backport for 3.0.2 as account_creation_restricted doesn't exist there.
Comment 6 Frédéric Buclin 2007-09-10 04:50:11 PDT
(In reply to comment #1)
> this is serious enough that we should release ASAP.

Agreed, for the reason given in comment 2 (installations are no longer private)!
Comment 7 Dave Miller [:justdave] (justdave@bugzilla.org) 2007-09-10 06:39:42 PDT
I still think we should move this check into Bugzilla::User::check_login_name_for_creation.  editusers can pass it an override bit to say "don't check this" when it's an admin doing the creation from there.  Making you do something extra to override it and having it checked by default otherwise no matter where you're checking from is much safer in the long run to prevent something like this from being introduced again by accident somewhere else.
Comment 8 Max Kanat-Alexander 2007-09-10 14:10:31 PDT
That has the potential of breaking other things, because it's not extremely simple in the Object->create framework (although it's definitely possible). I think we should stick with the simple fix for now and after the 3.0.2 and 3.1.2 release, we should move this stuff into check_login_name_for_creation.
Comment 9 Max Kanat-Alexander 2007-09-10 14:16:29 PDT
Created attachment 280385 [details] [diff] [review]
v1, 3.0

Here's the backport for 3.0.
Comment 10 Frédéric Buclin 2007-09-10 14:36:17 PDT
Comment on attachment 280385 [details] [diff] [review]
v1, 3.0

r=LpSolit
Comment 11 Frédéric Buclin 2007-09-11 09:02:16 PDT
Bah, I already complained in bug 350232 comment 5 a year ago about this security hole and you removed my r-! This security hole being here for a year, I suddenly see the release of 3.0.2 less urgent.
Comment 12 Max Kanat-Alexander 2007-09-18 16:30:31 PDT
tip:

Checking in Bugzilla/WebService/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Constants.pm,v  <--  Constants.pm
new revision: 1.10; previous revision: 1.9
done
Checking in Bugzilla/WebService/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/User.pm,v  <--  User.pm
new revision: 1.5; previous revision: 1.4
done

3.0:

Checking in Bugzilla/WebService/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Constants.pm,v  <--  Constants.pm
new revision: 1.6.2.2; previous revision: 1.6.2.1
done
Checking in Bugzilla/WebService/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/User.pm,v  <--  User.pm
new revision: 1.4.2.1; previous revision: 1.4
done
Comment 13 Max Kanat-Alexander 2007-09-18 19:15:22 PDT
Security Advisory sent, unlocking bug. (The website hasn't updated yet, and the announcement hasn't actually been cleared from the queue yet, but both should happen soon and I have to leave at the moment.)

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