From 27947d87c63ea5629c813e0622cf8c42602b9c3a Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Fri, 13 May 2022 10:17:44 -0230
Subject: [PATCH 1/2] 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
---
 jest.config.ts    |  4 ++--
 src/index.ts      | 32 ++++++++++++++++++++++++++++----
 static/index.css  | 22 ++++++++++++++--------
 static/index.html | 31 +++++++++++++++++++++++++++++++
 4 files changed, 75 insertions(+), 14 deletions(-)

diff --git a/jest.config.ts b/jest.config.ts
index 3ea32c2..60c16ed 100644
--- 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
index c1266f0..6d4b3e7 100644
--- 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
index 8498b62..c312613 100644
--- 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
index b443a1a..daf15f6 100644
--- 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>

From 0e02d1d1364459e8d01223784a3dffc6b12986e4 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Fri, 13 May 2022 14:07:44 -0230
Subject: [PATCH 2/2] Show subframe warning by default

This ensures that if JavaScript is disabled, we default to the subframe
view. If JavaScript is disabled, it usually indicates that the page is
in a subframe. This page can't work correctly without JavaScript
anyway.
---
 static/index.html | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/static/index.html b/static/index.html
index daf15f6..6961897 100644
--- a/static/index.html
+++ b/static/index.html
@@ -1,23 +1,15 @@
 <!DOCTYPE html>
 <html lang="en">
   <head>
-    <style id="antiClickjackSubFrameStyles">
-      .content__body {
-        display: none !important;
-      }
-    </style>
-    <style id="antiClickjackTopFrameStyles">
-      .content__framed-body {
+    <style id="antiClickjack">
+      #content__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);
+        document.getElementById('antiClickjack').innerHTML =
+          '#content__framed-body { display: none !important; }';
       }
     </script>
     <title>MetaMask Phishing Detection</title>
@@ -60,7 +52,7 @@ <h1>
          MetaMask Phishing Detection
         </h1>
       </div>
-      <div class="content__body">
+      <div id="content__body" class="content__body">
         <p>
           This domain is currently on the MetaMask domain warning list. This
           means that based on information available to us, MetaMask believes