Bug 1419417 - Parse HTML to make sure that tags and attributes are properly closed. r=mkmelin,jorgk a=jorgk
authorBen Bucksch <ben.bucksch@beonex.com>
Mon, 21 May 2018 18:44:41 +0200
changeset 28234 6eca16d60d9031dce68473eb3e1693116e4df3c1
parent 28233 4c8746e63c32a6389f501d052b6c2e63b2765192
child 28235 6b77778b904037f6f603a9424deb6e98cf3e1e97
push id2081
push usermozilla@jorgk.com
push dateTue, 22 May 2018 07:24:56 +0000
treeherdercomm-esr52@e3e80074c7e1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin, jorgk, jorgk
bugs1419417
Bug 1419417 - Parse HTML to make sure that tags and attributes are properly closed. r=mkmelin,jorgk a=jorgk This fixes the efail <http://efail.de> security bug, which opens a HTML tag or attribute in an HTML MIME part, then puts in a PGP-encrypted part, and then another HTML part with the closing quote or tag. This could be e.g. <img src=' or <form><textarea>, CSS URL or similar features that send out the following text as URL and therefore leak it to the attacker who crafted the email. The PGP part will then be decrypted and leak. The bug was that we just passed HTML through verbatim. The frontend does not have any further precautions, either. The correct solution here is to jail each MIME part into a separate <iframe type="content"> in the UI. However, we don't want one scrollbar for each MIME part, but one scroll for the entire body. <iframe seamless> would allow that, but it was never implemented in Firefox and is now dead. We might later find a workaround, but this is more work and can't be done short term. The fix here in libmime first parses the HTML that we get in the HTML MIME part, and then immediately serialized it again. That ensures that the HTML document is complete, syntactically correct, and all tags and attributes are properly closed, before we start with the next MIME part.
mailnews/mime/src/mimeTextHTMLParsed.cpp
mailnews/mime/src/mimeTextHTMLParsed.h
mailnews/mime/src/mimei.cpp
mailnews/mime/src/mimei.h
mailnews/mime/src/moz.build
new file mode 100644
--- /dev/null
+++ b/mailnews/mime/src/mimeTextHTMLParsed.cpp
@@ -0,0 +1,168 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/* Most of this code is copied from mimethsa. If you find a bug here, check that class, too. */
+
+/* This runs the entire HTML document through the Mozilla HTML parser, and
+   then outputs it as string again. This ensures that the HTML document is
+   syntactically correct and complete and all tags and attributes are closed.
+
+   That prevents "MIME in the middle" attacks like efail.de.
+   The base problem is that we concatenate different MIME parts in the output
+   and render them all together as a single HTML document in the display.
+
+   The better solution would be to put each MIME part into its own <iframe type="content">.
+   during rendering. Unfortunately, we'd need <iframe seamless> for that.
+   That would remove the need for this workaround, and stop even more attack classes.
+*/
+
+#include "mimeTextHTMLParsed.h"
+#include "prmem.h"
+#include "prlog.h"
+#include "msgCore.h"
+#include "nsIDOMParser.h"
+#include "nsIDOMDocument.h"
+#include "nsIDocument.h"
+#include "nsIDocumentEncoder.h"
+#include "mozilla/ErrorResult.h"
+#include "nsIPrefBranch.h"
+
+#define MIME_SUPERCLASS mimeInlineTextHTMLClass
+MimeDefClass(MimeInlineTextHTMLParsed, MimeInlineTextHTMLParsedClass,
+             mimeInlineTextHTMLParsedClass, &MIME_SUPERCLASS);
+
+static int MimeInlineTextHTMLParsed_parse_line(const char *, int32_t,
+                                               MimeObject *);
+static int MimeInlineTextHTMLParsed_parse_begin(MimeObject *obj);
+static int MimeInlineTextHTMLParsed_parse_eof(MimeObject *, bool);
+static void MimeInlineTextHTMLParsed_finalize(MimeObject *obj);
+
+static int
+MimeInlineTextHTMLParsedClassInitialize(MimeInlineTextHTMLParsedClass *clazz)
+{
+  MimeObjectClass *oclass = (MimeObjectClass *)clazz;
+  NS_ASSERTION(!oclass->class_initialized, "problem with superclass");
+  oclass->parse_line  = MimeInlineTextHTMLParsed_parse_line;
+  oclass->parse_begin = MimeInlineTextHTMLParsed_parse_begin;
+  oclass->parse_eof   = MimeInlineTextHTMLParsed_parse_eof;
+  oclass->finalize    = MimeInlineTextHTMLParsed_finalize;
+
+  return 0;
+}
+
+static int
+MimeInlineTextHTMLParsed_parse_begin(MimeObject *obj)
+{
+  MimeInlineTextHTMLParsed *me = (MimeInlineTextHTMLParsed *)obj;
+  me->complete_buffer = new nsString();
+  int status = ((MimeObjectClass*)&MIME_SUPERCLASS)->parse_begin(obj);
+  if (status < 0)
+    return status;
+
+  // Dump the charset we get from the mime headers into a HTML <meta http-equiv>.
+  char *content_type = obj->headers ?
+    MimeHeaders_get(obj->headers, HEADER_CONTENT_TYPE, false, false) : 0;
+  if (content_type)
+  {
+    char* charset = MimeHeaders_get_parameter(content_type,
+                                              HEADER_PARM_CHARSET,
+                                              NULL, NULL);
+    PR_Free(content_type);
+    if (charset)
+    {
+      nsAutoCString charsetline(
+        "\n<meta http-equiv=\"content-type\" content=\"text/html; charset=");
+      charsetline += charset;
+      charsetline += "\">\n";
+      int status = MimeObject_write(obj,
+                                    charsetline.get(),
+                                    charsetline.Length(),
+                                    true);
+      PR_Free(charset);
+      if (status < 0)
+        return status;
+    }
+  }
+  return 0;
+}
+
+static int
+MimeInlineTextHTMLParsed_parse_eof(MimeObject *obj, bool abort_p)
+{
+
+  if (obj->closed_p)
+    return 0;
+  int status = ((MimeObjectClass*)&MIME_SUPERCLASS)->parse_eof(obj, abort_p);
+  if (status < 0)
+    return status;
+  MimeInlineTextHTMLParsed *me = (MimeInlineTextHTMLParsed *)obj;
+
+  // We have to cache all lines and parse the whole document at once.
+  // There's a useful sounding function parseFromStream(), but it only allows XML
+  // mimetypes, not HTML. Methinks that's because the HTML soup parser
+  // needs the entire doc to make sense of the gibberish that people write.
+  if (!me || !me->complete_buffer)
+    return 0;
+
+  nsString& rawHTML = *(me->complete_buffer);
+  nsString parsed;
+  nsresult rv;
+
+  // Parse the HTML source.
+  nsCOMPtr<nsIDOMDocument> document;
+  nsCOMPtr<nsIDOMParser> parser = do_GetService(NS_DOMPARSER_CONTRACTID);
+  rv = parser->ParseFromString(rawHTML.get(), "text/html",
+                               getter_AddRefs(document));
+  NS_ENSURE_SUCCESS(rv, -1);
+
+  // Serialize it back to HTML source again.
+  nsCOMPtr<nsIDocumentEncoder> encoder = do_CreateInstance(
+    "@mozilla.org/layout/documentEncoder;1?type=text/html");
+  uint32_t aFlags = 0;
+  rv = encoder->Init(document, NS_LITERAL_STRING("text/html"), aFlags);
+  NS_ENSURE_SUCCESS(rv, -1);
+  rv = encoder->EncodeToString(parsed);
+  NS_ENSURE_SUCCESS(rv, -1);
+
+  // Write it out.
+  NS_ConvertUTF16toUTF8 resultCStr(parsed);
+  status = ((MimeObjectClass*)&MIME_SUPERCLASS)->parse_line(
+    resultCStr.BeginWriting(), resultCStr.Length(), obj);
+  rawHTML.Truncate();
+  return status;
+}
+
+void
+MimeInlineTextHTMLParsed_finalize(MimeObject *obj)
+{
+  MimeInlineTextHTMLParsed *me = (MimeInlineTextHTMLParsed *)obj;
+
+  if (me && me->complete_buffer)
+  {
+    obj->clazz->parse_eof(obj, false);
+    delete me->complete_buffer;
+    me->complete_buffer = NULL;
+  }
+
+  ((MimeObjectClass*)&MIME_SUPERCLASS)->finalize(obj);
+}
+
+static int
+MimeInlineTextHTMLParsed_parse_line(const char *line, int32_t length,
+                                    MimeObject *obj)
+{
+  MimeInlineTextHTMLParsed *me = (MimeInlineTextHTMLParsed *)obj;
+
+  if (!me || !(me->complete_buffer))
+    return -1;
+
+  nsCString linestr(line, length);
+  NS_ConvertUTF8toUTF16 line_ucs2(linestr.get());
+  if (length && line_ucs2.IsEmpty())
+    CopyASCIItoUTF16(linestr, line_ucs2);
+  (me->complete_buffer)->Append(line_ucs2);
+
+  return 0;
+}
new file mode 100644
--- /dev/null
+++ b/mailnews/mime/src/mimeTextHTMLParsed.h
@@ -0,0 +1,28 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef _MIMETEXTHTMLPARSED_H_
+#define _MIMETEXTHTMLPARSED_H_
+
+#include "mimethtm.h"
+
+typedef struct MimeInlineTextHTMLParsedClass MimeInlineTextHTMLParsedClass;
+typedef struct MimeInlineTextHTMLParsed      MimeInlineTextHTMLParsed;
+
+struct MimeInlineTextHTMLParsedClass {
+  MimeInlineTextHTMLClass html;
+};
+
+extern MimeInlineTextHTMLParsedClass mimeInlineTextHTMLParsedClass;
+
+struct MimeInlineTextHTMLParsed {
+  MimeInlineTextHTML    html;
+  nsString             *complete_buffer;  // Gecko parser expects wide strings
+};
+
+#define MimeInlineTextHTMLParsedClassInitializer(ITYPE,CSUPER) \
+  { MimeInlineTextHTMLClassInitializer(ITYPE,CSUPER) }
+
+#endif /* _MIMETEXTHTMLPARSED_H_ */
--- a/mailnews/mime/src/mimei.cpp
+++ b/mailnews/mime/src/mimei.cpp
@@ -34,16 +34,17 @@
 #include "mimeunty.h"  /*   |     |--- MimeUntypedText            */
 #include "mimeleaf.h"  /*   |--- MimeLeaf (abstract)            */
 #include "mimetext.h"  /*   |     |--- MimeInlineText (abstract)      */
 #include "mimetpla.h"  /*   |     |     |--- MimeInlineTextPlain      */
 #include "mimethpl.h"  /*   |     |     |     |--- M.I.TextHTMLAsPlaintext */
 #include "mimetpfl.h"   /*   |     |     |--- MimeInlineTextPlainFlowed     */
 #include "mimethtm.h"  /*   |     |     |--- MimeInlineTextHTML      */
 #include "mimethsa.h"  /*   |     |     |     |--- M.I.TextHTMLSanitized   */
+#include "mimeTextHTMLParsed.h" /*|     |     |--- M.I.TextHTMLParsed   */
 #include "mimetric.h"  /*   |     |     |--- MimeInlineTextRichtext    */
 #include "mimetenr.h"  /*   |     |     |     |--- MimeInlineTextEnriched  */
 /* SUPPORTED VIA PLUGIN      |     |     |--- MimeInlineTextVCard           */
 #include "mimeiimg.h"  /*   |     |--- MimeInlineImage            */
 #include "mimeeobj.h"  /*   |     |--- MimeExternalObject          */
 #include "mimeebod.h"  /*   |--- MimeExternalBody              */
                         /* If you add classes here,also add them to mimei.h */
 #include "prlog.h"
@@ -324,17 +325,17 @@ bool mime_is_allowed_class(const MimeObj
       );
 
   /* Contrairy to above, the below code is a "blacklist", i.e. it
      *excludes* some "bad" classes. */
   return
      !(
         (avoid_html
          && (
-              clazz == (MimeObjectClass *)&mimeInlineTextHTMLClass
+              clazz == (MimeObjectClass *)&mimeInlineTextHTMLParsedClass
                          /* Should not happen - we protect against that in
                             mime_find_class(). Still for safety... */
             )) ||
         (avoid_images
          && (
               clazz == (MimeObjectClass *)&mimeInlineImageClass
             )) ||
         (avoid_strange_content
@@ -505,21 +506,21 @@ mime_find_class (const char *content_typ
     else if (!PL_strncasecmp(content_type,      "text/", 5))
     {
       if      (!PL_strcasecmp(content_type+5,    "html"))
       {
         if (opts
             && opts->format_out == nsMimeOutput::nsMimeMessageSaveAs)
           // SaveAs in new modes doesn't work yet.
         {
-          clazz = (MimeObjectClass *)&mimeInlineTextHTMLClass;
+          clazz = (MimeObjectClass *)&mimeInlineTextHTMLParsedClass;
           types_of_classes_to_disallow = 0;
         }
         else if (html_as == 0 || html_as == 4) // Render sender's HTML
-          clazz = (MimeObjectClass *)&mimeInlineTextHTMLClass;
+          clazz = (MimeObjectClass *)&mimeInlineTextHTMLParsedClass;
         else if (html_as == 1) // convert HTML to plaintext
           // Do a HTML->TXT->HTML conversion, see mimethpl.h.
           clazz = (MimeObjectClass *)&mimeInlineTextHTMLAsPlaintextClass;
         else if (html_as == 2) // display HTML source
           /* This is for the freaks. Treat HTML as plaintext,
              which will cause the HTML source to be displayed.
              Not very user-friendly, but some seem to want this. */
           clazz = (MimeObjectClass *)&mimeInlineTextPlainClass;
@@ -911,32 +912,30 @@ mime_create (const char *content_type, M
                  ? MimeHeaders_get(hdrs, HEADER_CONTENT_DISPOSITION, true, false)
                  : 0);
   }
 
   if (!content_disposition || !PL_strcasecmp(content_disposition, "inline"))
     ;  /* Use the class we've got. */
   else
   {
-    //
-    // rhp: Ok, this is a modification to try to deal with messages
-    //      that have content disposition set to "attachment" even though
-    //      we probably should show them inline.
-    //
-    if (  (clazz != (MimeObjectClass *)&mimeInlineTextHTMLClass) &&
-          (clazz != (MimeObjectClass *)&mimeInlineTextClass) &&
+    // override messages that have content disposition set to "attachment"
+    // even though we probably should show them inline.
+    if (  (clazz != (MimeObjectClass *)&mimeInlineTextClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineTextPlainClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineTextPlainFlowedClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineTextHTMLClass) &&
+          (clazz != (MimeObjectClass *)&mimeInlineTextHTMLParsedClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineTextHTMLSanitizedClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineTextHTMLAsPlaintextClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineTextRichtextClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineTextEnrichedClass) &&
           (clazz != (MimeObjectClass *)&mimeMessageClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineImageClass) )
+      // not a special inline type, so show as attachment
       clazz = (MimeObjectClass *)&mimeExternalObjectClass;
   }
 
   /* If the option `Show Attachments Inline' is off, now would be the time to change our mind... */
   /* Also, if we're doing a reply (i.e. quoting the body), then treat that according to preference. */
   if (opts && ((!opts->show_attachment_inline_p && !forceInline) ||
                (!opts->quote_attachment_inline_p &&
                 (opts->format_out == nsMimeOutput::nsMimeMessageQuoting ||
--- a/mailnews/mime/src/mimei.h
+++ b/mailnews/mime/src/mimei.h
@@ -66,16 +66,18 @@
       |     |     +--- MimeInlineTextPlain
       |     |     |     |
       |     |     |     \--- MimeInlineTextHTMLAsPlaintext
       |     |     |
       |     |     +--- MimeInlineTextPlainFlowed
       |     |     |
       |     |     +--- MimeInlineTextHTML
       |     |     |     |
+      |     |     |     +--- MimeInlineTextHTMLParsed
+      |     |     |     |
       |     |     |     \--- MimeInlineTextHTMLSanitized
       |     |     |
       |     |     +--- MimeInlineTextRichtext
       |     |     |     |
       |     |     |     \--- MimeInlineTextEnriched
       |     |    |
       |     |    +--- MimeInlineTextVCard
       |     |
--- a/mailnews/mime/src/moz.build
+++ b/mailnews/mime/src/moz.build
@@ -50,16 +50,17 @@ SOURCES += [
     'mimemsg.cpp',
     'mimemsig.cpp',
     'mimemult.cpp',
     'mimeobj.cpp',
     'mimepbuf.cpp',
     'mimesun.cpp',
     'mimetenr.cpp',
     'mimetext.cpp',
+    'mimeTextHTMLParsed.cpp',
     'mimethpl.cpp',
     'mimethsa.cpp',
     'mimethtm.cpp',
     'mimetpfl.cpp',
     'mimetpla.cpp',
     'mimetric.cpp',
     'mimeunty.cpp',
     'nsCMS.cpp',