[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [bridgedb/master] Fix #12086; disallow emails not to our domain.



commit 5e6258041d492c0748e03079f366bbb0d06d53f9
Author: Isis Lovecruft <isis@xxxxxxxxxxxxxx>
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@xxxxxxxxxx'.
     * 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@xxxxxxxxxxxx'
         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@xxxxxxxxxx'
         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):



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits