Mail: Accounts not deleted when Nextcloud account is deleted

[anojht]

Hello, while testing we discovered that externally authenticated users with the same username but different domains have access to mailbox data from first user. This is after the user is first removed from the admin UI, before setting up the external authentication for a different domain with the same username.

This presents a huge security issue for users/admins who are using the mail app with external users authenticated to their instance should they ever decide to add other domains for external authentication or switch domains.

Steps to Reproduce

1. Setup external user support app with IMAP [email protected] for authentication.
2. Mail app auto sets up their mailbox once the user logs in.
3. Delete the user from admin UI.
4. Setup external user support app with IMAP [email protected] for authentication.
5. Login with the user and click the Mail app.

Expected behavior

Removing IMAP authenticated external user account should remove their mailbox data

Actual behavior

Mail app retains and displays the old user's mail account, folders, emails, and receives new emails alongside the new account.

Mail app

Mail app version: 0.11.0

Mailserver or service: Any email service

Server configuration

Operating system: unRAID 6.6.6

Web server: Nginx

Database: MariaDB

PHP version: 7.2.13

Nextcloud Version: 15.0.2

Client configuration

Browser: Chrome 71.0.3578.98

Operating system: Windows 10

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[ChristophWurst]

Setup external user support app with IMAP [email protected] for authentication

Are you talking about the IMAP user back-end? That is not provided by this app but https://github.com/nextcloud/user_external.

Mail app auto sets up their mailbox.

It does not unless you configure that. And even then it crates the automatic account dynamically, so if you change the config it will log the user into the new mail server automatically.

[ChristophWurst]

Just to clarify: are you talking about this feature https://github.com/nextcloud/mail/blob/master/doc/admin.md#automatic-account-creation?

[anojht]

Setup external user support app with IMAP [email protected] for authentication

Are you talking about the IMAP user back-end? That is not provided by this app but https://github.com/nextcloud/user_external.

Mail app auto sets up their mailbox.

It does not unless you configure that. And even then it crates the automatic account dynamically, so if you change the config it will log the user into the new mail server automatically.

I understand IMAP user back-end if provided by the the user_external app. However, we setup that app with the Mail app and we did not configure for automatic account creation. It just happened during testing after successful authentication and login.

Just to clarify: are you talking about this feature https://github.com/nextcloud/mail/blob/master/doc/admin.md#automatic-account-creation?

Yes this is the feature we are talking about. What happens is after we remove the user who authenticated and had their mailbox automatically configured. We changed the IMAP user back-end to point to a different domain but created a user in that domain with the same username as the previous domain. When we logged in this user had access to the previous mailbox, which appeared fully configured.

What should happen is, the Maill App should remove user mail data when the account is removed.

[anojht]

Sorry if this is confusing, I have edited the issue, hopefully this is more clear.

[ChristophWurst]

What should happen is, the Maill App should remove user mail data when the account is removed.

I think I know get what the issue is: you re-used the UID of the deleted user, right?

[anojht]

What should happen is, the Maill App should remove user mail data when the account is removed.

I think I know get what the issue is: you re-used the UID of the deleted user, right?

Yes we setup an email account on the new domain with the same UID as the previous domain and used this to login.
For ex.
[email protected]
[email protected]

[ChristophWurst]

Got it. I've updated the title accordingly.

In general: should there every be a security issue, do not report this in public issue trackers but use https://hackerone.com/nextcloud instead. Thanks :v:

[anojht]

Sorry, I did not know where to report security issues. I am not a pen tester. We were testing the Mail app for production roll-out of our shared hosting infrastructure. This seemed pretty important so I reported it in your app repo.

If you need more info what's the best way to contact you? We confirmed this to be an issue with the mail app because we inspected the tables Mail app creates in the DB.

[ChristophWurst]

No worries. This time it wasn't critical :wink:

It's fine for this bug.

[anojht]

Will this be fixed for 0.12? Kinda worried about this for existing users, not sure if they are aware of this? I am trying to figure out how I can contribute some code to get this fixed.

[ChristophWurst]

It will be fixed when someone submits a pull request.

[johanehnberg]

I think this may impact accounts created directly in NC, too? I can see oc_mail_accounts.user_id entries that have been deleted in there. And they still trigger background jobs.

I verified that there is at least no ON DELETE CASCADE rule for it. In fact, it looks like most of NC does not have that, only one app uses it in our env. I did not check deletion code.

[ChristophWurst]

https://github.com/nextcloud/mail/pull/3009

[johanehnberg]

How to correctly delete accounts affected by this bug retroactively? The fix only adds deletion of mail accounts on nextcloud user delete. There is no occ mail:delete command, and the database does not have proper cascading delete.

[ChristophWurst]

I'm afraid you have to do it manually and so this largely depends on the user back-end. For local users you can do with a simple SQL query where you drop all rows from oc_mail_account that have no matching entry in oc_accounts.

[johanehnberg]

Hmm, without the cascade rules, I am unsure how the other oc_mail_* tables will behave.

[ChristophWurst]

You don't have to worry about them. https://github.com/nextcloud/mail/blob/master/lib/BackgroundJob/CleanupJob.php will trigger https://github.com/nextcloud/mail/blob/769e3654636a9ed18e587bd1f6d0bce3363fc69c/lib/Service/CleanupService.php#L44-L49 and that will :fire: the rest.

[johanehnberg]

OK. So for posterity, the query is:

DELETE FROM oc_mail_accounts WHERE user_id NOT IN (SELECT a.uid FROM oc_accounts a);

[johanehnberg]

Adding this will trigger cleanup immediately if desired:
occ mail:clean-up