fix: Pass URLs to PhishingController (#2835)

Following MetaMask/metamask-extension#25839 full URLs are required as the argument for `PhishingController:testOrigin`. This PR makes sure that our calls to `isOnPhishingList` pass strictly full URLs.
MetaMask · Oct 14, 2024 · b78d274 · b78d274
1 parent 4be3dd3
commit b78d274
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 33 deletions.
diff --git a/packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx b/packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx
@@ -68,7 +68,7 @@ describe('SnapInterfaceController', () => {
       expect(rootMessenger.call).toHaveBeenNthCalledWith(
         3,
         'PhishingController:testOrigin',
-        'foo.bar',
+        'https://foo.bar/',
       );
 
       expect(content).toStrictEqual(getJsxElementFromComponent(components));
@@ -118,7 +118,7 @@ describe('SnapInterfaceController', () => {
       expect(rootMessenger.call).toHaveBeenNthCalledWith(
         3,
         'PhishingController:testOrigin',
-        'foo.bar',
+        'https://foo.bar/',
       );
 
       expect(content).toStrictEqual(element);
@@ -239,7 +239,7 @@ describe('SnapInterfaceController', () => {
       expect(rootMessenger.call).toHaveBeenNthCalledWith(
         3,
         'PhishingController:testOrigin',
-        'foo.bar',
+        'https://foo.bar/',
       );
     });
 
@@ -289,7 +289,7 @@ describe('SnapInterfaceController', () => {
       expect(rootMessenger.call).toHaveBeenNthCalledWith(
         3,
         'PhishingController:testOrigin',
-        'foo.bar',
+        'https://foo.bar/',
       );
     });
 
@@ -708,7 +708,7 @@ describe('SnapInterfaceController', () => {
       expect(rootMessenger.call).toHaveBeenNthCalledWith(
         5,
         'PhishingController:testOrigin',
-        'foo.bar',
+        'https://foo.bar/',
       );
     });
 
@@ -774,7 +774,7 @@ describe('SnapInterfaceController', () => {
       expect(rootMessenger.call).toHaveBeenNthCalledWith(
         5,
         'PhishingController:testOrigin',
-        'foo.bar',
+        'https://foo.bar/',
       );
     });
 

diff --git a/packages/snaps-utils/coverage.json b/packages/snaps-utils/coverage.json
@@ -1,6 +1,6 @@
 {
   "branches": 99.74,
   "functions": 98.92,
-  "lines": 99.45,
-  "statements": 96.31
+  "lines": 99.46,
+  "statements": 96.32
 }
diff --git a/packages/snaps-utils/src/ui.test.tsx b/packages/snaps-utils/src/ui.test.tsx
@@ -550,38 +550,41 @@ describe('validateLink', () => {
     expect(() => validateLink('mailto:foo@bar.com', fn, fn)).not.toThrow();
 
     expect(fn).toHaveBeenCalledTimes(2);
-    expect(fn).toHaveBeenCalledWith('foo.bar');
-    expect(fn).toHaveBeenCalledWith('bar.com');
+    expect(fn).toHaveBeenCalledWith('https://foo.bar/');
+    expect(fn).toHaveBeenCalledWith('https://bar.com');
   });
 
   it('passes for a valid list of emails', () => {
     const fn = jest.fn().mockReturnValue(false);
+    const getSnap = jest.fn();
 
     expect(() =>
-      validateLink('mailto:foo@bar.com,bar@baz.com,baz@qux.com', fn),
+      validateLink('mailto:foo@bar.com,bar@baz.com,baz@qux.com', fn, getSnap),
     ).not.toThrow();
 
     expect(fn).toHaveBeenCalledTimes(3);
-    expect(fn).toHaveBeenCalledWith('bar.com');
-    expect(fn).toHaveBeenCalledWith('baz.com');
-    expect(fn).toHaveBeenCalledWith('qux.com');
+    expect(fn).toHaveBeenCalledWith('https://bar.com');
+    expect(fn).toHaveBeenCalledWith('https://baz.com');
+    expect(fn).toHaveBeenCalledWith('https://qux.com');
   });
 
   it('passes for a valid email with a parameter', () => {
     const fn = jest.fn().mockReturnValue(false);
+    const getSnap = jest.fn();
 
     expect(() =>
-      validateLink('mailto:foo@bar.com?subject=Subject', fn),
+      validateLink('mailto:foo@bar.com?subject=Subject', fn, getSnap),
     ).not.toThrow();
 
     expect(fn).toHaveBeenCalledTimes(1);
-    expect(fn).toHaveBeenCalledWith('bar.com');
+    expect(fn).toHaveBeenCalledWith('https://bar.com');
   });
 
   it('throws an error for an invalid protocol', () => {
     const fn = jest.fn().mockReturnValue(false);
+    const getSnap = jest.fn();
 
-    expect(() => validateLink('http://foo.bar', fn, fn)).toThrow(
+    expect(() => validateLink('http://foo.bar', fn, getSnap)).toThrow(
       'Invalid URL: Protocol must be one of: https:, mailto:, metamask:.',
     );
 
@@ -620,7 +623,7 @@ describe('validateLink', () => {
     ).toThrow('Invalid URL: The specified URL is not allowed.');
 
     expect(fn).toHaveBeenCalledTimes(1);
-    expect(fn).toHaveBeenCalledWith('test.metamask-phishing.io');
+    expect(fn).toHaveBeenCalledWith('https://test.metamask-phishing.io/');
   });
 
   it('throws an error for a phishing email', () => {
@@ -631,45 +634,52 @@ describe('validateLink', () => {
     ).toThrow('Invalid URL: The specified URL is not allowed.');
 
     expect(fn).toHaveBeenCalledTimes(1);
-    expect(fn).toHaveBeenCalledWith('test.metamask-phishing.io');
+    expect(fn).toHaveBeenCalledWith('https://test.metamask-phishing.io');
   });
 
   it('throws an error for a phishing email when using multiple emails', () => {
     const fn = jest.fn().mockImplementation((email) => {
-      if (email === 'test.metamask-phishing.io') {
+      if (email === 'https://test.metamask-phishing.io') {
         return true;
       }
 
       return false;
     });
+    const getSnap = jest.fn();
 
     expect(() =>
-      validateLink('mailto:foo@test.metamask-phishing.io,foo@bar.com', fn),
+      validateLink(
+        'mailto:foo@test.metamask-phishing.io,foo@bar.com',
+        fn,
+        getSnap,
+      ),
     ).toThrow('Invalid URL: The specified URL is not allowed.');
 
     expect(fn).toHaveBeenCalledTimes(1);
-    expect(fn).toHaveBeenCalledWith('test.metamask-phishing.io');
+    expect(fn).toHaveBeenCalledWith('https://test.metamask-phishing.io');
   });
 
   it('throws an error for a phishing email when using parameters', () => {
     const fn = jest.fn().mockImplementation((email) => {
-      if (email === 'test.metamask-phishing.io') {
+      if (email === 'https://test.metamask-phishing.io') {
         return true;
       }
 
       return false;
     });
+    const getSnap = jest.fn();
 
     expect(() =>
       validateLink(
         'mailto:foo@bar.com,foo@test.metamask-phishing.io?subject=Subject',
         fn,
+        getSnap,
       ),
     ).toThrow('Invalid URL: The specified URL is not allowed.');
 
     expect(fn).toHaveBeenCalledTimes(2);
-    expect(fn).toHaveBeenCalledWith('bar.com');
-    expect(fn).toHaveBeenCalledWith('test.metamask-phishing.io');
+    expect(fn).toHaveBeenCalledWith('https://bar.com');
+    expect(fn).toHaveBeenCalledWith('https://test.metamask-phishing.io');
   });
 });
 

diff --git a/packages/snaps-utils/src/ui.tsx b/packages/snaps-utils/src/ui.tsx
@@ -360,19 +360,15 @@ export function validateLink(
       const emails = url.pathname.split(',');
       for (const email of emails) {
         const hostname = email.split('@')[1];
-        assert(
-          !isOnPhishingList(hostname),
-          'The specified URL is not allowed.',
-        );
+        assert(!hostname.includes(':'));
+        const href = `https://${hostname}`;
+        assert(!isOnPhishingList(href), 'The specified URL is not allowed.');
       }
 
       return;
     }
 
-    assert(
-      !isOnPhishingList(url.hostname),
-      'The specified URL is not allowed.',
-    );
+    assert(!isOnPhishingList(url.href), 'The specified URL is not allowed.');
   } catch (error) {
     throw new Error(
       `Invalid URL: ${