Prevent clickjacking

When the page is loaded in a sub-frame, we now display a condensed warning and a link to open the page in a new tab. It is no longer possible to add a site to the safelist from an iframe. This ensures that clickjacking cannot be used to add a site to the safelist without a users knowledge or consent. The technique used was adapted from the OWASP clickjacking defense cheat sheet [1]. This technique is recommended for legacy browsers that do not support security headers. We are using this technique instead of security headers because GitHub Pages does not allow using security headers. The consequence of failure for this defense is low; all it would allow is bypassing our phishing warning. The page used to bypass the warning would itself remain susceptible to blocking. As such, it's unclear what advantage such a bypass would have over moving to a new domain that is not yet blocked. This low consequence of failure is the reason I am recommending the use of this "legacy browser" technique, rather than moving to an alternate hosting platform that supports modern security headers. This technique appears to work in all browsers, and that's good enough for now. We can move to an alternative hosting platform that supports security headers at a later date if necessary. [1]: https://cheatsheetseries.owasp.org/cheatsheets/Clickjacking_Defense_Cheat_Sheet.html#best-for-now-legacy-browser-frame-breaking-script
MetaMask · May 13, 2022 · 4be6bef · 4be6bef
1 parent 076c779
commit 4be6bef
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 14 deletions.
diff --git a/jest.config.ts b/jest.config.ts
@@ -44,8 +44,8 @@ const config: Config.InitialOptions = {
     global: {
       branches: 20,
       functions: 50,
-      lines: 67,
-      statements: 67,
+      lines: 66,
+      statements: 66,
     },
   },
 

diff --git a/src/index.ts b/src/index.ts
@@ -45,11 +45,35 @@ window.addEventListener('load', async () => {
   }
 });
 
-// Skip stream initialization on extension startup, when this page is loaded
-// in a hidden iframe. No need to setup streams to handle user interaction in
-// that case.
+// Skip stream initialization on extension startup (when this page is loaded
+// in a hidden iframe), and in sub-frames. In both cases, the user interactions
+// handled by the streams are not possible.
 if (!isExtensionStartup()) {
-  window.document.addEventListener('DOMContentLoaded', start);
+  if (window.top === window.self) {
+    window.document.addEventListener('DOMContentLoaded', start);
+  } else {
+    // The sub-frame case requires the "open in new tab" href to be set
+    // dynamically because a relative `href` attribute would not preserve
+    // the URL hash.
+    window.document.addEventListener(
+      'DOMContentLoaded',
+      setupOpenSelfInNewTabLink,
+    );
+  }
+}
+
+/**
+ * Setup the "Open in new tab" link.
+ *
+ * This is necessary so that the "open in new tab" link includes the current
+ * URL hash. A statically-set relative `href` would drop the URL hash.
+ */
+function setupOpenSelfInNewTabLink() {
+  const newTabLink = window.document.getElementById('open-self-in-new-tab');
+  if (!newTabLink) {
+    throw new Error('Unable to locate "Open in new tab" link');
+  }
+  newTabLink.setAttribute('href', window.location.href);
 }
 
 /**

diff --git a/static/index.css b/static/index.css
@@ -36,6 +36,16 @@ body {
   font-family: Euclid, Roboto, Helvetica, Arial, sans-serif;
 }
 
+p {
+  margin: 2em;
+}
+
+p a {
+  text-decoration: underline;
+  color: var(--color-primary-default);
+  cursor: pointer;
+}
+
 .content {
   display: flex;
   flex-direction: column;
@@ -81,12 +91,8 @@ body {
   color: var(--color-text-default);
 }
 
-.content__body p {
-  margin: 2em;
-}
-
-.content__body p a {
-  text-decoration: underline;
-  color: var(--color-primary-default);
-  cursor: pointer;
+.content__framed-body {
+  background-color: var(--color-background-alternative);
+  font-size: 1rem;
+  color: var(--color-text-default);
 }
diff --git a/static/index.html b/static/index.html
@@ -1,6 +1,25 @@
 <!DOCTYPE html>
 <html lang="en">
   <head>
+    <style id="antiClickjackSubFrameStyles">
+      .content__body {
+        display: none !important;
+      }
+    </style>
+    <style id="antiClickjackTopFrameStyles">
+      .content__framed-body {
+        display: none !important;
+      }
+    </style>
+    <script type="text/javascript">
+      if (self === top) {
+        const antiClickjackSubFrameStyles = document.getElementById('antiClickjackSubFrameStyles');
+        antiClickjackSubFrameStyles.parentNode.removeChild(antiClickjackSubFrameStyles);
+      } else {
+        const antiClickjackTopFrameStyles = document.getElementById('antiClickjackTopFrameStyles');
+        antiClickjackTopFrameStyles.parentNode.removeChild(antiClickjackTopFrameStyles)
+      }
+    </script>
     <title>MetaMask Phishing Detection</title>
     <script
       src="./globalthis.js"
@@ -85,6 +104,18 @@ <h1>
           >.
         </p>
       </div>
+      <div id="content__framed-body" class="content__framed-body">
+        <p>
+          This domain is currently on the MetaMask domain warning list. This
+          means that based on information available to us, MetaMask believes
+          this domain could currently compromise your security and, as an added
+          safety feature, MetaMask has restricted access to the site.
+        </p>
+        <p>
+          <a id="open-self-in-new-tab" target="_blank">Open this warning in a new tab</a> for more information
+          on why this domain is blocked, and how to continue at your own risk.
+        </p>
+      </div>
     </div>
   </body>
 </html>