395632 - [SECURITY] XML-RPC WebService Bugzilla::User::offer_account_by_email does not check createemailregexp

Status: RESOLVED FIXED

(Bugzilla :: WebService, defect)

Description

[Sascha Jensen]

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/[e-mail]

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Yeah, that's my fault, and this is serious enough that we should release ASAP.

Comment 2

[Max Kanat-Alexander]

This compromises requirelogin installations.

Comment 3

[Marc Schumann [:Wurblzap]]

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]

(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]

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]

(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]]

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]

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]

Attachment: v1, 3.0

Here's the backport for 3.0.

Comment 10

[Frédéric Buclin]

Comment on attachment 280385 [details] [diff] [review]
v1, 3.0

r=LpSolit

Comment 11

[Frédéric Buclin]

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]

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]

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.)