Remove attributes that contain javascript from MHTML

third_party/WebKit/Source/web/WebFrameSerializer.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 63 matching lines @@
 namespace blink {
 
 namespace {
 
 class MHTMLFrameSerializerDelegate final : public FrameSerializer::Delegate {
   WTF_MAKE_NONCOPYABLE(MHTMLFrameSerializerDelegate);
 
  public:
   explicit MHTMLFrameSerializerDelegate(
       WebFrameSerializer::MHTMLPartsGenerationDelegate&);
-  bool shouldIgnoreAttribute(const Attribute&) override;
+  bool shouldIgnoreAttribute(const Element&, const Attribute&) override;
   bool rewriteLink(const Element&, String& rewrittenLink) override;
   bool shouldSkipResourceWithURL(const KURL&) override;
   bool shouldSkipResource(const Resource&) override;
 
  private:
   WebFrameSerializer::MHTMLPartsGenerationDelegate& m_webDelegate;
 };
 
 MHTMLFrameSerializerDelegate::MHTMLFrameSerializerDelegate(
     WebFrameSerializer::MHTMLPartsGenerationDelegate& webDelegate)
     : m_webDelegate(webDelegate) {}
 
 bool MHTMLFrameSerializerDelegate::shouldIgnoreAttribute(
+    const Element& element,
     const Attribute& attribute) {
   // TODO(fgorski): Presence of srcset attribute causes MHTML to not display
   // images, as only the value of src is pulled into the archive. Discarding
   // srcset prevents the problem. Long term we should make sure to MHTML plays
   // nicely with srcset.
-  return attribute.localName() == HTMLNames::srcsetAttr;
+  if (attribute.localName() == HTMLNames::srcsetAttr)
+    return true;
+
+  // Do not include attributes that can contain javascript:

[Łukasz Anforowicz, 2016/11/29 18:59:55]

nit: Could you please expand the comment to explain that we do this because
opening a mhtml won't execute any javascript for security reasons, and so we can
save disk space by stripping the unnecessary attributes.

[jianli, 2016/11/30 00:46:26]

Done.

+  // 1) Any event handler attribute.
+  // 2) Any attribute that can contain a URL will be executed as Javascript.

[Łukasz Anforowicz, 2016/11/29 18:59:55]

nit: I am not sure if the 2 comment lines above are useful - they just repeat
what the code below says.  I would consider removing them.  WDYT?

[jianli, 2016/11/30 00:46:26]

Done.

+  return Element::isEventHandlerAttribute(attribute) ||
+         element.isJavaScriptURLAttribute(attribute);
 }
 
 bool MHTMLFrameSerializerDelegate::rewriteLink(const Element& element,
                                                String& rewrittenLink) {
   if (!element.isFrameOwnerElement())
     return false;
 
   auto* frameOwnerElement = toHTMLFrameOwnerElement(&element);
   Frame* frame = frameOwnerElement->contentFrame();
   if (!frame)
@@ skipping 194 matching lines @@
     const WebString& baseTarget) {
   // TODO(yosin) We should call |FrameSerializer::baseTagDeclarationOf()|.
   if (baseTarget.isEmpty())
     return String("<base href=\".\">");
   String baseString = "<base href=\".\" target=\"" +
                       static_cast<const String&>(baseTarget) + "\">";
   return baseString;
 }
 
 }  // namespace blink