1194987 - Editing your email address and make it point to a non-existent email address makes Bugzilla stop working

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect)

Description

[Teemu Mannermaa (:wicked)]

It seems to be possible to cripple a Bugzilla install by adding user with an invalid email address. Invalid address can be either syntactically incorrect or by using non-existent user with a local domain. Not sure if the same happens with remote domains.

I've marked this as security initially as this seems to be a way to cause a denial of service for a Bugzilla install. I don't know what a remote user did or didn't do to make this happen so I'm not sure how severe or easy this is to exploit. Also, not sure how to fix this. Probably needs direct DB editing. :(

Landfill 5.0 branch currently demonstrates this error and any attempt to access any page fails with:

--!--
An unexpected error occurred. This could be a temporary problem, or some code is behaving incorrectly. If this problem persists, please email this page to [e-mail] with details of what you were doing at the time this message appeared.

URL: https://landfill.bugzilla.org/bugzilla-5.0-branch/

There was an error sending mail from '[e-mail]' to '[e-mail]': 5.1.1 <[e-mail]>... User unknown
Traceback:

 at Bugzilla/Mailer.pm line 186.
	Bugzilla::Mailer::MessageToMTA(...) called at Bugzilla/Mailer.pm line 227
	Bugzilla::Mailer::send_staged_mail(...) called at Bugzilla/DB.pm line 1223
	Bugzilla::DB::bz_commit_transaction(...) called at Bugzilla/Object.pm line 540
	Bugzilla::Object::update(...) called at Bugzilla/User.pm line 221
	Bugzilla::User::update(...) called at Bugzilla/Auth/Verify.pm line 123
	Bugzilla::Auth::Verify::create_or_update_user(...) called at Bugzilla/Auth/Verify/Stack.pm line 71
	Bugzilla::Auth::Verify::Stack::create_or_update_user(...) called at Bugzilla/Auth.pm line 67
	Bugzilla::Auth::login(...) called at Bugzilla.pm line 311
	Bugzilla::login(...) called at /var/www/html/bugzilla-5.0-branch/index.cgi line 21
--!--

Anybody with access to landfill is free to try figure out what's wrong with the install and how to fix it.

Comment 1

[Frédéric Buclin]

This is not a blocker nor a security bug. First of all, you need a valid email address to activate your account, as you need the URL in the email to enter your password. Invalid email addresses were only possible till Bugzilla 2.22, see bug 87795. Admins can still create invalid accounts, but in that case, they are shooting themselves in the foot.

Another story is when a previously valid email address is no longer used. But that's already covered by bug 599890.

Comment 2

[Frédéric Buclin]

Fixed. I cleared the mail_staging DB table which contained this invalid email address.

Comment 3

[Frédéric Buclin]

I also ran sanitycheck.cgi and deleted the 14 accounts with an invalid email address. Their user ID was old, so these accounts were probably created before landfill has been upgraded to Bugzilla 3.0.

Comment 4

[Frédéric Buclin]

OK, I managed to understand what went wrong exactly, and it's pretty bad.

First step: a user wants to change his email address from userprefs.cgi. He chooses one which doesn't exist but which is syntaxically correct. As all changes made from userprefs.cgi happen within a transaction, the email to confirm the email change is stored in the mail_staging DB table and is sent right after. But the MTA is unable to deliver this email because the recipient doesn't exist:

 There was an error sending mail from '[e-mail]' to '[e-mail]': 5.1.1 <[e-mail]>... User unknown

Traceback:

 at Bugzilla/Mailer.pm line 186.
	Bugzilla::Mailer::MessageToMTA(...) called at Bugzilla/Mailer.pm line 227
	Bugzilla::Mailer::send_staged_mail(...) called at Bugzilla/DB.pm line 1223
	Bugzilla::DB::bz_commit_transaction(...) called at /var/www/html/bugzilla-5.0-branch/userprefs.cgi line 128
	main::SaveAccount(...) called at /var/www/html/bugzilla-5.0-branch/userprefs.cgi line 609


Because Bugzilla crashes, the email with the non-existent recipient remains in the mail_staging table.


Second step: *every time* a user tries to access Bugzilla, it will immediately crash independently of the page you visit, because all pages start with:

  Bugzilla->login(LOGIN_OPTIONAL);

or

  Bugzilla->login(LOGIN_REQUIRED);

to know who the user is, to correctly display the page header and footer with the correct links (Administration, saved searches, etc...) and to determine if the user is allowed to access this page or not. But this triggers:

 There was an error sending mail from '[e-mail]' to '[e-mail]': 5.1.1 <[e-mail]>... User unknown

Traceback:

 at Bugzilla/Mailer.pm line 186.
	Bugzilla::Mailer::MessageToMTA(...) called at Bugzilla/Mailer.pm line 227
	Bugzilla::Mailer::send_staged_mail(...) called at Bugzilla/DB.pm line 1223
	Bugzilla::DB::bz_commit_transaction(...) called at Bugzilla/Object.pm line 540
	Bugzilla::Object::update(...) called at Bugzilla/User.pm line 221
	Bugzilla::User::update(...) called at Bugzilla/Auth/Verify.pm line 123
	Bugzilla::Auth::Verify::create_or_update_user(...) called at Bugzilla/Auth/Verify/Stack.pm line 71
	Bugzilla::Auth::Verify::Stack::create_or_update_user(...) called at Bugzilla/Auth.pm line 67
	Bugzilla::Auth::login(...) called at Bugzilla.pm line 311
	Bugzilla::login(...) called at /var/www/html/bugzilla-5.0-branch/index.cgi line 21


The reason is that Bugzilla::Auth::Verify::create_or_update_user() *always* call $user->update() for the ultra rare cases where the login name or real name changed. This triggers a call to Bugzilla::Object->update which must start a transaction to avoid race conditions. When the transaction ends, it will clear the mail_staging table and Bugzilla will try to send the email with the non-existent recipient again, causing the MTA to crash as shown above.


This affects Bugzilla 4.5.6 and above.

Comment 5

[Frédéric Buclin]

Attachment: patch, v1

This patch does several things (it applies to both 5.0 and master):

- Fix Bugzilla/Auth/Verify.pm to stop calling $user->update when no changes are made. This prevents Bugzilla from trying to clear the email queue at login.

- Fix Bugzilla/Mailer.pm to stop spamming the same users again and again. Currently, it only clears the mail_staging table once *all* emails have been sent. But if one of the recipient no longer exists, MessageToMTA() will throw an error before it had a chance to clear already sent emails. Those users get duplicates everytime someone tries to access Bugzilla. (In my testing on landfill, I got 20 spam in less than 10 minutes due to that, and another user probably got hundreds of them before I found what the problem was.)

- Fix Bugzilla::Token::IssueEmailChangeToken() and issue_new_user_account_token() to not put confirmation emails in the queue. In case the recipient doesn't exist, this would block the whole queue as MessageToMTA() would throw an error. Instead, it sends emails immediately, which has the desired side-effect to revert the email address change if the new email address doesn't exist. I also did some cleanup here because some old and useless code was there since Bugzilla 2.16, see bug 23067.

- If the old email address no longer exists (and which would be the reason why the user wants to change his email address in Bugzilla), then we cannot send the email and we display the URL in the page itself instead, so that the user still has a chance to cancel the process if necessary. Of course, if the MTA doesn't get the "Delivery Status Notification (Failure)" message on time, the user would have no way to cancel the process. But that's already the case currently.

Comment 6

[Daniel Veditz [:dveditz]]

This seems quite different from the bug you tried to dupe this too. In that one the non-working email addresses are skipped and left in the queue but bugzilla doesn't crash. This bug would seem to make it easy for a malicious person to keep a public instance like BMO offline by repeatedly changing account email addresses every time we got it back up. At least until we disabled new accounts and changing emails on existing accounts (in case they prepared for the first fix by pre-creating a bunch of throw-away accounts), but that would be pretty disruptive and could only be a short-term fix.

Comment 7

[Mark Côté [:mcote]]

We don't know if this affects BMO.  Frédéric says it affects only Bugzilla 4.5.6 and above.  The core of BMO is still running 4.2, so this would only be an issue if the relevant code was already backported to BMO.  Maybe glob can check.

Comment 8

[:glob ✱]

(In reply to Mark Côté [:mcote] from comment #7)
> We don't know if this affects BMO.

this does not impact bmo.

Comment 9

[Gervase Markham [:gerv]]

(In reply to Frédéric Buclin from comment #5)
> - Fix Bugzilla/Auth/Verify.pm to stop calling $user->update when no changes
> are made. This prevents Bugzilla from trying to clear the email queue at
> login.

Why does updating a user clear the mail queue anyway?

> - Fix Bugzilla/Mailer.pm to stop spamming the same users again and again.
> Currently, it only clears the mail_staging table once *all* emails have been
> sent. But if one of the recipient no longer exists, MessageToMTA() will
> throw an error before it had a chance to clear already sent emails.

You have indeed avoided the multispam, but the code still doesn't manage to continue with the rest of the mails if one of them is bogus. How about doing that?

> - Fix Bugzilla::Token::IssueEmailChangeToken() and
> issue_new_user_account_token() to not put confirmation emails in the queue.
> In case the recipient doesn't exist, this would block the whole queue as
> MessageToMTA() would throw an error. Instead, it sends emails immediately,
> which has the desired side-effect to revert the email address change if the
> new email address doesn't exist. I also did some cleanup here because some
> old and useless code was there since Bugzilla 2.16, see bug 23067.

This will work some of the time, but some errors in email delivery are not synchronous. So it's not guaranteed that the code will detect that the new address is bogus.

> - If the old email address no longer exists (and which would be the reason
> why the user wants to change his email address in Bugzilla), then we cannot
> send the email and we display the URL in the page itself instead, so that
> the user still has a chance to cancel the process if necessary.

Again, we can't know for certain if the old address no longer exists, so why not print the "cancel" or "undo" link in the web page unconditionally?

Gerv

Comment 10

[Frédéric Buclin]

Attachment: patch, v2

I discussed with gerv on IRC about issues he raised. This patch fixes the remaining point we discussed.

Comment 11

[Gervase Markham [:gerv]]

Comment on attachment 8655985 [details] [diff] [review]
patch, v2

Review of attachment 8655985 [details] [diff] [review]:
-----------------------------------------------------------------

r=gerv.

Gerv

Comment 12

[Frédéric Buclin]

To ssh://[e-mail]/bugzilla/bugzilla.git
   a666b2a..2380717  master -> master

To ssh://[e-mail]/bugzilla/bugzilla.git
   e76030a..a52e66e  5.0 -> 5.0