Skip to content

fix: Prevent malicious dapps from spoofing origin in PermissionsSummary #14422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from

Conversation

smilingkylan
Copy link
Contributor

@smilingkylan smilingkylan commented Apr 4, 2025

Description

The purpose of this task is to fix a security vulnerability whereby a malicious dapp can trick the user into thinking that a permissions request is coming from a different ("trusted") website rather than their malicious dapp origin

Related issues

Fixes: https://github.com/MetaMask/mobile-planning/issues/2096

Manual testing steps

  1. Build the following codespace: https://github.com/MetaMask/temp-poc-templates-repo/tree/pm-security/2096?tab=readme-ov-file
  2. In MetaMask, use the in-app browser to navigate to the URL where that codespace is hosted, and authorize the browser to access the codespace
  3. When you open the malicious website, a permissions request will pop up. Before the fix, it would eventually show https://portfolio.metamask.io but with the new fix it should still show the codespace URL

Screenshots/Recordings

Before

Sorry it's so slow!

android-spoofing-before.mov

After

android-spoofing-after.mov

Pre-merge author checklist

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.

Copy link
Contributor

github-actions bot commented Apr 4, 2025

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@smilingkylan smilingkylan marked this pull request as ready for review April 4, 2025 00:34
@smilingkylan smilingkylan requested a review from a team as a code owner April 4, 2025 00:34
@smilingkylan smilingkylan added team-mobile-platform Mobile Platform team Run Smoke E2E Triggers smoke e2e on Bitrise labels Apr 4, 2025
@MetaMask MetaMask deleted a comment from sonarqubecloud bot Apr 4, 2025
Copy link
Contributor

github-actions bot commented Apr 4, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: c9bd0f2
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6ee8b742-b886-49ce-a06d-fb4502d24cc8

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@smilingkylan smilingkylan requested review from a team as code owners April 4, 2025 06:39
@smilingkylan smilingkylan added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Apr 4, 2025
Copy link
Contributor

github-actions bot commented Apr 4, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 88942a7
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/09e50321-d3ae-4fe4-a590-7002b67d1040

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2025
@smilingkylan
Copy link
Contributor Author

Closing in lieu of #13394

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Run Smoke E2E Triggers smoke e2e on Bitrise team-mobile-platform Mobile Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant