[geary/wip/791275-mailsploit-mitigation: 4/8] Ensure mailbox addresses escaped correctly when formatted as RFC 822.

[Michael Gratton <mjog src gnome org>]

commit 8810e9575352e49a086c779558d4e3ea74045d98
Author: Michael James Gratton <mike vee net>
Date:   Tue Jan 30 22:33:36 2018 +1030

    Ensure mailbox addresses escaped correctly when formatted as RFC 822.
    
    * src/engine/rfc822/rfc822-mailbox-address.vala (MailboxAddress): Escape
      and encode name and mailbox (local-part) when serialising as a RFC 822
      string. Provide a means to get a RFC 822 version of the address only
      for SMTP. Add unit tests.
    
    * src/engine/smtp/smtp-request.vala (MailRequest): Use proper RFC 822
      formatted version of mailbox addresses.

 src/engine/rfc822/rfc822-mailbox-address.vala |   67 ++++++++++++++++++++-----
 src/engine/smtp/smtp-request.vala             |    2 +-
 test/engine/rfc822-mailbox-address-test.vala  |   62 +++++++++++++++++++++--
 3 files changed, 111 insertions(+), 20 deletions(-)
---
diff --git a/src/engine/rfc822/rfc822-mailbox-address.vala b/src/engine/rfc822/rfc822-mailbox-address.vala
index c4de959..26df61e 100644
--- a/src/engine/rfc822/rfc822-mailbox-address.vala
+++ b/src/engine/rfc822/rfc822-mailbox-address.vala
@@ -9,6 +9,11 @@
 /**
  * An immutable representation of an RFC 822 mailbox address.
  *
+ * The properties of this class such as {@link name} and {@link
+ * address} are stores decoded UTF-8, thus they must be re-encoded
+ * using methods such as {@link to_rfc822_string} before being re-used
+ * in a message envelope.
+ *
  * See [[https://tools.ietf.org/html/rfc5322#section-3.4]]
  */
 public class Geary.RFC822.MailboxAddress : Geary.MessageData.SearchableMessageData,
@@ -31,40 +36,52 @@ public class Geary.RFC822.MailboxAddress : Geary.MessageData.SearchableMessageDa
 
 
     internal delegate string ListToStringDelegate(MailboxAddress address);
-    
+
+
     /**
-     * The optional user-friendly name associated with the {@link MailboxAddress}.
+     * The optional human-readable part of the mailbox address.
      *
      * For "Dirk Gently <dirk example com>", this would be "Dirk Gently".
+     *
+     * The returned value has been decoded into UTF-8.
      */
     public string? name { get; private set; }
-    
+
     /**
      * The routing of the message (optional, obsolete).
+     *
+     * The returned value has been decoded into UTF-8.
      */
     public string? source_route { get; private set; }
-    
+
     /**
-     * The mailbox (local-part) portion of the {@link MailboxAddress}.
+     * The mailbox (local-part) portion of the mailbox's address.
      *
      * For "Dirk Gently <dirk example com>", this would be "dirk".
+     *
+     * The returned value has been decoded into UTF-8.
      */
     public string mailbox { get; private set; }
-    
+
     /**
-     * The domain portion of the {@link MailboxAddress}.
+     * The domain portion of the mailbox's address.
      *
      * For "Dirk Gently <dirk example com>", this would be "example.com".
+     *
+     * The returned value has been decoded into UTF-8.
      */
     public string domain { get; private set; }
-    
+
     /**
-     * The address specification of the {@link MailboxAddress}.
+     * The complete address part of the mailbox address.
      *
      * For "Dirk Gently <dirk example com>", this would be "dirk example com".
+     *
+     * The returned value has been decoded into UTF-8.
      */
     public string address { get; private set; }
 
+
     public MailboxAddress(string? name, string address) {
         this.name = name;
         this.address = address;
@@ -293,14 +310,38 @@ public class Geary.RFC822.MailboxAddress : Geary.MessageData.SearchableMessageDa
     }
 
     /**
-     * Returns the address suitable for insertion into an RFC822 message.
+     * Returns the complete mailbox address, armoured for RFC 822 use.
+     *
+     * This method is similar to {@link to_full_display}, but only
+     * checks for a distinct address (per Postel's Law) and not for
+     * any spoofing, and does not strip extra white space or
+     * non-printing characters.
      *
-     * @return the RFC822 quoted form of the full address.
+     * @return the RFC822 encoded form of the full address.
      */
     public string to_rfc822_string() {
         return has_distinct_name()
-            ? "%s <%s>".printf(GMime.utils_quote_string(this.name), this.address)
-            : this.address;
+            ? "%s <%s>".printf(
+                GMime.utils_header_encode_phrase(this.name),
+                to_rfc822_address()
+            )
+            : to_rfc822_address();
+    }
+
+    /**
+     * Returns the address part only, armoured for RFC 822 use.
+     *
+     * @return the RFC822 encoded form of the address, without angle
+     * brackets.
+     */
+    public string to_rfc822_address() {
+        return "%s@%s".printf(
+            // XXX utils_quote_string won't quote if spaces or quotes
+            // present, so need to do that manually
+            GMime.utils_quote_string(GMime.utils_header_encode_text(this.mailbox)),
+            // XXX Need to punycode international domains.
+            this.domain
+        );
     }
 
     /**
diff --git a/src/engine/smtp/smtp-request.vala b/src/engine/smtp/smtp-request.vala
index c3db013..fccf435 100644
--- a/src/engine/smtp/smtp-request.vala
+++ b/src/engine/smtp/smtp-request.vala
@@ -58,7 +58,7 @@ public class Geary.Smtp.EhloRequest : Geary.Smtp.Request {
 
 public class Geary.Smtp.MailRequest : Geary.Smtp.Request {
     public MailRequest(Geary.RFC822.MailboxAddress from) {
-        base (Command.MAIL, { "from:%s".printf(from.to_address_display("<", ">")) });
+        base (Command.MAIL, { "from:<%s>".printf(from.to_rfc822_address()) });
     }
 
     public MailRequest.plain(string addr) {
diff --git a/test/engine/rfc822-mailbox-address-test.vala b/test/engine/rfc822-mailbox-address-test.vala
index 172bdda..a7fe5a8 100644
--- a/test/engine/rfc822-mailbox-address-test.vala
+++ b/test/engine/rfc822-mailbox-address-test.vala
@@ -10,10 +10,13 @@ class Geary.RFC822.MailboxAddressTest : Gee.TestCase {
     public MailboxAddressTest() {
         base("Geary.RFC822.MailboxAddressTest");
         add_test("is_valid_address", is_valid_address);
+        add_test("unescaped_constructor", unescaped_constructor);
         add_test("is_spoofed", is_spoofed);
         add_test("has_distinct_name", has_distinct_name);
         add_test("to_full_display", to_full_display);
         add_test("to_short_display", to_short_display);
+        // latter depends on the former, so test that first
+        add_test("to_rfc822_address", to_rfc822_address);
         add_test("to_rfc822_string", to_rfc822_string);
     }
 
@@ -35,6 +38,34 @@ class Geary.RFC822.MailboxAddressTest : Gee.TestCase {
         assert(Geary.RFC822.MailboxAddress.is_valid_address("") == false);
     }
 
+    public void unescaped_constructor() {
+        MailboxAddress addr1 = new MailboxAddress("test1", "test2 example com");
+        assert(addr1.name == "test1");
+        assert(addr1.address == "test2 example com");
+        assert(addr1.mailbox == "test2");
+        assert(addr1.domain == "example.com");
+
+        MailboxAddress addr2 = new MailboxAddress(null, "test1@test2 example com");
+        assert(addr2.address == "test1@test2 example com");
+        assert(addr2.mailbox == "test1@test2");
+        assert(addr2.domain == "example.com");
+
+        MailboxAddress addr3 = new MailboxAddress(null, "©@example.com");
+        assert(addr3.address == "©@example.com");
+        assert(addr3.mailbox == "©");
+        assert(addr3.domain == "example.com");
+
+        MailboxAddress addr4 = new MailboxAddress(null, "😸@example.com");
+        assert(addr4.address == "😸@example.com");
+        assert(addr4.mailbox == "😸");
+        assert(addr4.domain == "example.com");
+
+        MailboxAddress addr5 = new MailboxAddress(null, "example.com");
+        assert(addr5.address == "example.com");
+        assert(addr5.mailbox == "");
+        assert(addr5.domain == "");
+    }
+
     public void is_spoofed() {
         assert(new MailboxAddress(null, "example example com").is_spoofed() == false);
         assert(new MailboxAddress("", "example example com").is_spoofed() == false);
@@ -91,6 +122,23 @@ class Geary.RFC822.MailboxAddressTest : Gee.TestCase {
                "example@example example com");
     }
 
+    public void to_rfc822_address() {
+        assert(new MailboxAddress(null, "example example com").to_rfc822_address() ==
+               "example example com");
+        //assert(new MailboxAddress(null, "test test example com").to_rfc822_address() ==
+        //       "\"test test\"@example.com");
+        //assert(new MailboxAddress(null, "test\" test example com").to_rfc822_address() ==
+        //       "\"test\" test\"@example.com");
+        //assert(new MailboxAddress(null, "test\"test example com").to_rfc822_address() ==
+        //       "\"test\"test\"@example.com");
+        assert(new MailboxAddress(null, "test@test example com").to_rfc822_address() ==
+               "\"test@test\"@example.com");
+        assert(new MailboxAddress(null, "©@example.com").to_rfc822_address() ==
+               "\"=?iso-8859-1?b?qQ==?=\"@example.com");
+        assert(new MailboxAddress(null, "😸@example.com").to_rfc822_address() ==
+               "\"=?UTF-8?b?8J+YuA==?=\"@example.com");
+    }
+
     public void to_rfc822_string() {
         assert(new MailboxAddress("", "example example com").to_rfc822_string() ==
                "example example com");
@@ -102,14 +150,16 @@ class Geary.RFC822.MailboxAddressTest : Gee.TestCase {
                "test test <example example com>");
         assert(new MailboxAddress("example example com", "example example com").to_rfc822_string() ==
                "example example com");
-        // Technically, per
-        // https://tools.ietf.org/html/rfc5322#appendix-A.1.2 this
-        // would be fine as just "test? <example example com>",
-        // i.e. without the name being quoted, but I guess GMime is
-        // just being conservative here?
         assert(new MailboxAddress("test?", "example example com").to_rfc822_string() ==
-               "\"test?\" <example example com>");
+               "test? <example example com>");
+        assert(new MailboxAddress("test@test", "example example com").to_rfc822_string() ==
+               "\"test@test\" <example example com>");
         assert(new MailboxAddress(";", "example example com").to_rfc822_string() ==
                "\";\" <example example com>");
+        assert(new MailboxAddress("©", "example example com").to_rfc822_string() ==
+               "=?iso-8859-1?b?qQ==?= <example example com>");
+        assert(new MailboxAddress("😸", "example example com").to_rfc822_string() ==
+               "=?UTF-8?b?8J+YuA==?= <example example com>");
     }
+
 }