Skip to content

Commit

Permalink
fix: Add main frame URL property to req object whenever req is trigge…
Browse files Browse the repository at this point in the history
…red from an iframe (#29337)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

See the attached issue in metamask planning for more details.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29337?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to `https://develop.d3bkcslj57l47p.amplifyapp.com/`
2. Click on Proceed anyways (This phishing warning page here is
expected)
3. Open the network tab to monitor network requests
4. Connect your wallet and click on a signature or transaction
5. Verify that mainFrameOrigin is included in the payload of the network
request to the security alerts API

<img width="1727" alt="Screenshot 2024-12-20 at 10 46 05 AM"
src="https://github.com/user-attachments/assets/71a0868d-21cf-4ce2-af20-11f092beb2ce"
/>

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

Below are screenshots demonstrating the behavior of a test HTML page I
created:

1. In the first screenshot, before the iframe is loaded, the console
shows only the origin of the main frame.
2. In the second screenshot, after clicking the button to load an iframe
pointing to example.com, the solution correctly identifies both the
mainFrameOrigin (main frame) and the origin (iframe).

<img width="1728" alt="Screenshot 2024-12-18 at 10 24 48 PM"
src="https://github.com/user-attachments/assets/244a1f9a-a0c1-4c82-b89e-8b20a8238d8e"
/>


<img width="1728" alt="Screenshot 2024-12-18 at 10 24 54 PM"
src="https://github.com/user-attachments/assets/ac28aacd-ec4a-4fd0-b644-8564345ea3d4"
/>




### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
AugmentedMode authored Dec 20, 2024
1 parent d510d5c commit 695d0db
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
24 changes: 24 additions & 0 deletions app/scripts/lib/createMainFrameOriginMiddleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Request and responses are currently untyped.
/* eslint-disable @typescript-eslint/no-explicit-any */

/**
* Returns a middleware that appends the mainFrameOrigin to request
*
* @param {{ mainFrameOrigin: string }} opts - The middleware options
* @returns {Function}
*/

export default function createMainFrameOriginMiddleware({
mainFrameOrigin,
}: {
mainFrameOrigin: string;
}) {
return function mainFrameOriginMiddleware(
req: any,
_res: any,
next: () => void,
) {
req.mainFrameOrigin = mainFrameOrigin;
next();
};
}
22 changes: 21 additions & 1 deletion app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ import {
createUnsupportedMethodMiddleware,
} from './lib/rpc-method-middleware';
import createOriginMiddleware from './lib/createOriginMiddleware';
import createMainFrameOriginMiddleware from './lib/createMainFrameOriginMiddleware';
import createTabIdMiddleware from './lib/createTabIdMiddleware';
import { NetworkOrderController } from './controllers/network-order';
import { AccountOrderController } from './controllers/account-order';
Expand Down Expand Up @@ -5804,11 +5805,18 @@ export default class MetamaskController extends EventEmitter {
tabId = sender.tab.id;
}

let mainFrameOrigin = origin;
if (sender.tab && sender.tab.url) {
// If sender origin is an iframe, then get the top-level frame's origin
mainFrameOrigin = new URL(sender.tab.url).origin;
}

const engine = this.setupProviderEngineEip1193({
origin,
sender,
subjectType,
tabId,
mainFrameOrigin,
});

const dupeReqFilterStream = createDupeReqFilterStream();
Expand Down Expand Up @@ -5929,13 +5937,25 @@ export default class MetamaskController extends EventEmitter {
* @param {MessageSender | SnapSender} options.sender - The sender object.
* @param {string} options.subjectType - The type of the sender subject.
* @param {tabId} [options.tabId] - The tab ID of the sender - if the sender is within a tab
* @param {mainFrameOrigin} [options.mainFrameOrigin] - The origin of the main frame if the sender is an iframe
*/
setupProviderEngineEip1193({ origin, subjectType, sender, tabId }) {
setupProviderEngineEip1193({
origin,
subjectType,
sender,
tabId,
mainFrameOrigin,
}) {
const engine = new JsonRpcEngine();

// Append origin to each request
engine.push(createOriginMiddleware({ origin }));

// Append mainFrameOrigin to each request if present
if (mainFrameOrigin) {
engine.push(createMainFrameOriginMiddleware({ mainFrameOrigin }));
}

// Append selectedNetworkClientId to each request
engine.push(createSelectedNetworkMiddleware(this.controllerMessenger));

Expand Down

0 comments on commit 695d0db

Please sign in to comment.