[tor-commits] [bridgedb/master] Fix #12086; disallow emails not to our domain.
isis at torproject.org
isis at torproject.org
Fri Jun 6 23:39:14 UTC 2014
commit 5e6258041d492c0748e03079f366bbb0d06d53f9
Author: Isis Lovecruft <isis at torproject.org>
Date: Wed May 28 23:05:20 2014 +0000
Fix #12086; disallow emails not to our domain.
At the SMTP layer, we previously only checked that the email address
within the SMTP 'RCPT TO:' command (after being stripped of any '+'
aliases) as well as the email address in the 'To:' header both had a
username equal to the configured EMAIL_USERNAME. It didn't check that
these email addresses were the same, nor did it check the domain at all.
This refactors the ``bridgedb.email.server.MailDelivery.validateTo()``
method to check that the domain name in the SMTP 'RCPT TO:' command is
either our domain or a subdomain of it.
At the email layer, we now check that the email was sent to a domain
which is either our domain or a subdomain of it. We also check that the
username matches our configured username (after '+' aliases have been
removed).
* FIXES #12086, a bug where BridgeDB would accept emails addressed to
addresses such as 'givemebridges at serious.ly'.
* CHANGES MailDelivery.validateTo() to check domain names and usernames
in the SMTP 'RCPT TO' command that an incoming email was received
for.
* RENAME MailMessage.getRecipient() → MailMessage.getMailFrom() in
bridgedb.email.server module.
* CHANGES MailMessage.getMailFrom() to check domain names and usernames
in the 'To:' header of an incoming email.
---
lib/bridgedb/email/server.py | 71 ++++++++++++++++++++++++--------
lib/bridgedb/test/test_email_server.py | 18 ++++----
2 files changed, 62 insertions(+), 27 deletions(-)
diff --git a/lib/bridgedb/email/server.py b/lib/bridgedb/email/server.py
index 79b4a59..593713b 100644
--- a/lib/bridgedb/email/server.py
+++ b/lib/bridgedb/email/server.py
@@ -506,7 +506,7 @@ class MailMessage(object):
else:
return client
- def getRecipient(self, incoming):
+ def getMailFrom(self, incoming):
"""Find our address in the recipients list of the **incoming** message.
:type incoming: :api:`twisted.mail.smtp.rfc822.Message`
@@ -516,21 +516,42 @@ class MailMessage(object):
:return: Our address from the recipients list. If we can't find it
return our default ``SMTP_FROM_ADDRESS`` from the config file.
"""
- ourAddress = self.context.fromAddr
+ logging.debug("Searching for our email address in 'To:' header...")
+
+ ours = None
try:
- ourAddress = smtp.Address(ourAddress)
+ ourAddress = smtp.Address(self.context.fromAddr)
allRecipients = incoming.getaddrlist("To")
+
for _, addr in allRecipients:
recipient = smtp.Address(addr)
- # See if the user looks familiar. We do a 'find' instead of
- # compare because we might have a '+' address here.
- if recipient.local.find(ourAddress.local) != -1:
- return '@'.join([recipient.local, recipient.domain])
- except smtp.AddressError as error:
- logging.warn(error)
+ if not (ourAddress.domain in recipient.domain):
+ logging.debug(("Not our domain (%s) or subdomain, skipping"
+ " email address: %s")
+ % (ourAddress.domain, str(recipient)))
+ continue
+ # The recipient's username should at least start with ours,
+ # but it still might be a '+' address.
+ if not recipient.local.startswith(ourAddress.local):
+ logging.debug(("Username doesn't begin with ours, skipping"
+ " email address: %s") % str(recipient))
+ continue
+ # Ignore everything after the first '+', if there is one.
+ beforePlus = recipient.local.split('+', 1)[0]
+ if beforePlus == ourAddress.local:
+ ours = str(recipient)
+ if not ours:
+ raise BadEmail(allRecipients)
- return ourAddress
+ except Exception as error:
+ logging.error(("Couldn't find our email address in incoming email "
+ "headers: %r" % error))
+ # Just return the email address that we're configured to use:
+ ours = self.context.fromAddr
+
+ logging.debug("Found our email address: %s." % ours)
+ return ours
def getCanonicalDomain(self, domain):
try:
@@ -571,7 +592,7 @@ class MailMessage(object):
d.addErrback(_replyEB)
incoming = self.getIncomingMessage()
- recipient = self.getRecipient(incoming)
+ recipient = self.getMailFrom(incoming)
client = self.getClientAddress(incoming)
if not client:
@@ -693,13 +714,27 @@ class MailDelivery(object):
:returns: A parameterless function which returns an instance of
:class:`SMTPMessage`.
"""
- u = user.dest.local
- # Hasplus? If yes, strip '+foo'
- idx = u.find('+')
- if idx != -1:
- u = u[:idx]
- if u != self.context.username:
- raise smtp.SMTPBadRcpt(user)
+ logging.debug("Validating SMTP 'RCPT TO:' email address...")
+
+ recipient = user.dest
+ ourAddress = smtp.Address(self.context.smtpFromAddr)
+
+ if not (ourAddress.domain in recipient.domain):
+ logging.debug(("Not our domain (%s) or subdomain, skipping"
+ " SMTP 'RCPT TO' address: %s")
+ % (ourAddress.domain, str(recipient)))
+ raise smtp.SMTPBadRcpt(str(recipient))
+ # The recipient's username should at least start with ours,
+ # but it still might be a '+' address.
+ if not recipient.local.startswith(ourAddress.local):
+ logging.debug(("Username doesn't begin with ours, skipping"
+ " SMTP 'RCPT TO' address: %s") % str(recipient))
+ raise smtp.SMTPBadRcpt(str(recipient))
+ # Ignore everything after the first '+', if there is one.
+ beforePlus = recipient.local.split('+', 1)[0]
+ if beforePlus != ourAddress.local:
+ raise smtp.SMTPBadRcpt(str(recipient))
+
return lambda: MailMessage(self.context, self.fromCanonical)
diff --git a/lib/bridgedb/test/test_email_server.py b/lib/bridgedb/test/test_email_server.py
index 7015339..7aff08e 100644
--- a/lib/bridgedb/test/test_email_server.py
+++ b/lib/bridgedb/test/test_email_server.py
@@ -316,37 +316,37 @@ class MailMessageTests(unittest.TestCase):
]
self.message.lines = lines
- def test_MailMessage_getRecipient_notbridgedb_at_yikezors_dot_net(self):
- """MailMessage.getRecipient() for an incoming email sent to any email
+ def test_MailMessage_getMailFrom_notbridgedb_at_yikezors_dot_net(self):
+ """MailMessage.getMailFrom() for an incoming email sent to any email
address other than the one we're listening for should return our
configured address, not the one in the incoming email.
"""
self._getIncomingLines()
self.message.lines[1] = 'To: notbridgedb at yikezors.net'
incoming = self.message.getIncomingMessage()
- recipient = self.message.getRecipient(incoming)
+ recipient = str(self.message.getMailFrom(incoming))
self.assertEqual(recipient, self.context.fromAddr)
- def test_MailMessage_getRecipient_givemebridges_at_seriously(self):
- """MailMessage.getRecipient() for an incoming email sent to any email
+ def test_MailMessage_getMailFrom_givemebridges_at_seriously(self):
+ """MailMessage.getMailFrom() for an incoming email sent to any email
address other than the one we're listening for should return our
configured address, not the one in the incoming email.
"""
self._getIncomingLines()
self.message.lines[1] = 'To: givemebridges at serious.ly'
incoming = self.message.getIncomingMessage()
- recipient = self.message.getRecipient(incoming)
+ recipient = str(self.message.getMailFrom(incoming))
self.assertEqual(recipient, self.context.fromAddr)
- def test_MailMessage_getRecipient_bad_address(self):
- """MailMessage.getRecipient() for an incoming email sent to a malformed
+ def test_MailMessage_getMailFrom_bad_address(self):
+ """MailMessage.getMailFrom() for an incoming email sent to a malformed
email address should log an smtp.AddressError and then return our
configured email address.
"""
self._getIncomingLines()
self.message.lines[1] = 'To: ><@><<<>>.foo'
incoming = self.message.getIncomingMessage()
- recipient = self.message.getRecipient(incoming)
+ recipient = str(self.message.getMailFrom(incoming))
self.assertEqual(recipient, self.context.fromAddr)
def test_MailMessage_reply_noFrom(self):
More information about the tor-commits
mailing list