fix: prevent mailbox disclosure (#2284)

This commit is contained in:
Carlos Quintana 2024-10-23 10:24:15 +02:00 committed by GitHub
parent 1afd392e5c
commit f55ab58d0c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 20 additions and 5 deletions

View file

@ -121,10 +121,16 @@ def mailbox_route():
@login_required @login_required
def mailbox_verify(): def mailbox_verify():
mailbox_id = request.args.get("mailbox_id") mailbox_id = request.args.get("mailbox_id")
if not mailbox_id:
LOG.i("Missing mailbox_id")
flash("You followed an invalid link", "error")
return redirect(url_for("dashboard.mailbox_route"))
code = request.args.get("code") code = request.args.get("code")
if not code: if not code:
# Old way # Old way
return verify_with_signed_secret(mailbox_id) return verify_with_signed_secret(mailbox_id)
try: try:
mailbox = mailbox_utils.verify_mailbox_code(current_user, mailbox_id, code) mailbox = mailbox_utils.verify_mailbox_code(current_user, mailbox_id, code)
except mailbox_utils.MailboxError as e: except mailbox_utils.MailboxError as e:

View file

@ -171,17 +171,17 @@ def verify_mailbox_code(user: User, mailbox_id: int, code: str) -> Mailbox:
f"User {user} failed to verify mailbox {mailbox_id} because it does not exist" f"User {user} failed to verify mailbox {mailbox_id} because it does not exist"
) )
raise MailboxError("Invalid mailbox") raise MailboxError("Invalid mailbox")
if mailbox.user_id != user.id:
LOG.i(
f"User {user} failed to verify mailbox {mailbox_id} because it's owned by another user"
)
raise MailboxError("Invalid mailbox")
if mailbox.verified: if mailbox.verified:
LOG.i( LOG.i(
f"User {user} failed to verify mailbox {mailbox_id} because it's already verified" f"User {user} failed to verify mailbox {mailbox_id} because it's already verified"
) )
clear_activation_codes_for_mailbox(mailbox) clear_activation_codes_for_mailbox(mailbox)
return mailbox return mailbox
if mailbox.user_id != user.id:
LOG.i(
f"User {user} failed to verify mailbox {mailbox_id} because it's owned by another user"
)
raise MailboxError("Invalid mailbox")
activation = ( activation = (
MailboxActivation.filter(MailboxActivation.mailbox_id == mailbox_id) MailboxActivation.filter(MailboxActivation.mailbox_id == mailbox_id)

View file

@ -286,6 +286,15 @@ def test_verify_other_users_mailbox():
mailbox_utils.verify_mailbox_code(user, mailbox.id, "9999999") mailbox_utils.verify_mailbox_code(user, mailbox.id, "9999999")
def test_verify_other_users_already_verified_mailbox():
other = create_new_user()
mailbox = Mailbox.create(
user_id=other.id, email=random_email(), verified=True, commit=True
)
with pytest.raises(mailbox_utils.MailboxError):
mailbox_utils.verify_mailbox_code(user, mailbox.id, "9999999")
@mail_sender.store_emails_test_decorator @mail_sender.store_emails_test_decorator
def test_verify_fail(): def test_verify_fail():
output = mailbox_utils.create_mailbox(user, random_email()) output = mailbox_utils.create_mailbox(user, random_email())