Commit 3d880c56 authored by legoktm's avatar legoktm
Browse files

Check a user owns the email they are trying to unsubscribe (CVE-2021-40347)

The list unsubscribe/ endpoint now performs validation that the user
making the request owns the email address they have requested be
unsubscribed. Without this check, any logged-in user could unsubscribe
any other email address from any list, also leaking whether that address
was subscribed in the first place.

Closes #531.
parent 1a460f92
......@@ -88,6 +88,12 @@ Ascessibility
* Move the focus to the textarea in mass subscribe page if there are errors in
the form. (Closes #493)
Security
--------
* Check that a user owns the email address they are trying to unsubscribe. This
fixes a bug in which any logged-in user could unsubscribe any email address
from any mailing list, leaking whether that address was subscribed originally.
(CVE-2021-40347, Closes #531)
.. _news-1.3.4:
......
......@@ -33,8 +33,13 @@ class TestListJoinLeave(ViewTestCase):
self.foo_list = self.domain.create_list('foo')
self.user = User.objects.create_user(
'testuser', 'test@example.com', 'testpass')
self.user2 = User.objects.create_user(
'testuser2', 'test2@example.com', 'test2pass')
EmailAddress.objects.create(
user=self.user, email=self.user.email, verified=True, primary=True)
EmailAddress.objects.create(
user=self.user2, email=self.user2.email, verified=True,
primary=True)
def test_unsubscribe_pending_verification(self):
"""Test that unsubscription waiting verification shows right message"""
......@@ -71,6 +76,32 @@ class TestListJoinLeave(ViewTestCase):
'You have a pending unsubscription request waiting for'
' moderator approval.')
def test_unsubscribe_self_only(self):
"""Test that a user can unsubscribe themselves, but not other users"""
for address in ['test@example.com', 'test2@example.com']:
self.foo_list.subscribe(address, pre_approved=True,
pre_verified=True, pre_confirmed=True)
self.client.force_login(user=self.user)
response = self.client.post(reverse('list_unsubscribe',
args=[self.foo_list.list_id]),
data={'email': 'test@example.com'},
follow=True)
self.assertEqual(response.status_code, 200)
self.assertContains(response, 'test@example.com has been '
'unsubscribed from this list.')
# They should no longer be a member
self.assertRaises(ValueError,
lambda: self.foo_list.get_member('test@example.com'))
# Now have user test try to unsubscribe user test2
response = self.client.post(reverse('list_unsubscribe',
args=[self.foo_list.list_id]),
data={'email': 'test2@example.com'},
follow=True)
self.assertEqual(response.status_code, 200)
self.assertContains(response, "You can only unsubscribe yourself.")
# Should still be a member
self.assertIsNotNone(self.foo_list.get_member('test2@example.com'))
def test_subscribe_to_lists(self):
self.client.force_login(self.user)
res = self.client.post(reverse('list_subscribe',
......
......@@ -553,6 +553,15 @@ class ListUnsubscribeView(MailingListView):
@method_decorator(login_required)
def post(self, request, *args, **kwargs):
email = request.POST['email']
# Verify the user actually controls this email, should
# return 1 if the user owns the email, 0 otherwise.
found_email = EmailAddress.objects.filter(
user=request.user, email=email, verified=True).count()
if found_email == 0:
messages.error(
request,
_('You can only unsubscribe yourself.'))
return redirect('list_summary', self.mailing_list.list_id)
if self._has_pending_unsub_req(email):
messages.error(
request,
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment