Seems to me that the check is obsolete now that scripts are disabled in MHTML.  Łukasz, what do you think?

There are some related discussion in bug https://bugs.chromium.org/p/chromium/issues/detail?id=126955

+jcivelli@ as he is the author of the "For security reasons we only load MHTML pages from the local file system" check

+dcheng@ as he has more experience than me on the security team and might hopefully help with the discussion of XSS risks associated with allowing opening MHTML from the web (rather than only from local files)


I don't understand what is meant by "now that scripts are disabled in MHTML" :-(  When saving HTML into MHTML, scripts (i.e. contents of body.onload or span.onclick attribute) are preserved (I've just checked on ToT).  OTOH, when opening the saved MHTML file I indeed didn't see the alert / the effects of script execution.  I don't understand why this happened / why javascript didn't get executed.  Could you help me find code that vets javascript in MHTML files?


In parallel I tried to understand what security-sensitive scenarios we were concerned above when we introduced the check in MHTMLArchive::create.  Unfortunately I was only able to find the following:

- The check for only loading MHTML from "local" URIs has been present in MHTMLAchive.cpp since the first commit - https://chromium.googlesource.com/chromium/src/+/c780cb02dc1cd621d0777e0bfca53ebd5a74286d

- Some questions have been asked about the security check during code review at https://bugs.webkit.org/show_bug.cgi?id=7168 ("Interesting.  Is that true for the other archive formats?"), but they haven't been answered.  


Without understanding scenarios the check protects against, I am not sure if I can confidently say that without javascript execution we are safe (despite MHTML's ability to present arbitrary content as originating from a given site/domain).  There might be other, non-javascript-related ways for unexpected content to be harmful (i.e. if javascript from main frame trusts dom contents of subframes, then if one subframe can influence content of other, cross-origin subframes maybe bad things can happen).

Re scripting: we don't allow scripts to run in MHTML pages, because the MHTML format allows a resource to specify its origin. A malicious MHTML file could abuse this to steal local storage, etc.

I'm not really familiar with Android's content URIs, so I'm not sure how safe it is to always consider them as local resources. CCing some people who might know more...

I also did some code archaeology: it looks like this check landed with the original patch for MTHML support: https://bugs.webkit.org/show_bug.cgi?id=7168

and was tweaked in https://bugs.webkit.org/show_bug.cgi?id=86540 to use SchemeRegistry.

1. As dcheng@ alludes to, MHTML has some interesting properties that make it dangerous. Without a more thorough audit of the system, I'm reluctant to consider MHTML URLs as anything other than malicious and dangerous. So, I'd like to keep the check.

2. Setting Android's content URLs as local would have a number of side effects. It would be good to run through the callsites of `SecurityOrigin::isLocal()` (which would start returning `true`) and `SecurityOrigin::canDisplay()` (which would start returning `false` pretty often).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77ce66b313e37fd0bba77d3f85769e593c10a353

commit 77ce66b313e37fd0bba77d3f85769e593c10a353
Author: weiran <weiran@google.com>
Date: Wed Aug 31 22:09:58 2016

Allow loading MHTML archive from content scheme

BUG= 630753 

Review-Url: https://codereview.chromium.org/2247063002
Cr-Commit-Position: refs/heads/master@{#415775}

[modify] https://crrev.com/77ce66b313e37fd0bba77d3f85769e593c10a353/third_party/WebKit/Source/platform/mhtml/MHTMLArchive.cpp
[modify] https://crrev.com/77ce66b313e37fd0bba77d3f85769e593c10a353/third_party/WebKit/Source/platform/mhtml/MHTMLArchive.h
[modify] https://crrev.com/77ce66b313e37fd0bba77d3f85769e593c10a353/third_party/WebKit/Source/web/tests/MHTMLTest.cpp