Skip to content
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

Prevent brave wallet providers from being generated in third party iframe #13268

Merged
merged 1 commit into from
May 10, 2022

Conversation

darkdh
Copy link
Member

@darkdh darkdh commented May 9, 2022

Resolves brave/brave-browser#22686

  1. window.ethereum and window.solana will be unavailable in 3rd party iframe
  2. Some browser test cleanup

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

Same party test

  1. Navigate to https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe
  2. Add id="test" to iframe element and click Run
  3. Open console and inspect any element at the right panel
  4. Select iframe result in console like this

Screen Shot 2022-05-09 at 15 13 50

5. Type these in console
  let iframe = document.getElementById('test')
  console.log(iframe.contentWindow.ethereum)
  1. It should not be undefined

Third party test

  1. Navigate to https://www.w3schools.com/tags/tryit.asp?filename=tryhtml_iframe
  2. Add id="test" to iframe element and change src to https://metamask.github.io/test-dapp/ and click Run
  3. Open console and inspect any element at the right panel
  4. Select iframe result in console like this

Screen Shot 2022-05-09 at 15 13 50

5. Type these in console
  let iframe = document.getElementById('test')
  console.log(iframe.contentWindow.ethereum)
  1. It should be undefined

@darkdh darkdh requested review from diracdeltas and yrliou May 9, 2022 21:44
@darkdh darkdh requested a review from a team as a code owner May 9, 2022 21:44
@darkdh darkdh self-assigned this May 9, 2022
@@ -45,6 +45,10 @@ void BraveWalletRenderFrameObserver::DidCreateScriptContext(
js_ethereum_provider_.reset();
return;
}
// Wallet provider objects won't be generated for third party iframe
if (!render_frame()->IsMainFrame() && !render_frame()->GetMainRenderFrame()) {
Copy link
Member

@yrliou yrliou May 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably better to use render_frame()->GetWebFrame()->IsCrossOriginToMainFrame() to check if it's 3rd party iframe.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds better, fixed and force-pushed

@darkdh darkdh force-pushed the block-3p-iframe-provider branch from 542851a to 75f935c Compare May 9, 2022 22:48
Copy link
Member

@yrliou yrliou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@diracdeltas diracdeltas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx for doing this

@darkdh darkdh added this to the 1.40.x - Nightly milestone May 9, 2022
@darkdh darkdh merged commit 92462c7 into master May 10, 2022
@darkdh darkdh deleted the block-3p-iframe-provider branch May 10, 2022 01:29
darkdh added a commit that referenced this pull request May 10, 2022
Prevent brave wallet providers from being generated in third party iframe
@srirambv
Copy link
Contributor

Verification passed on

Brave 1.40.44 Chromium: 101.0.4951.54 (Official Build) nightly (64-bit)
Revision 67da1aeb32cedd27634ca6634fb79cbd85d3f0ab-refs/branch-heads/4951@{#1126}
OS Windows 11 Version 22H2 (Build 22616.1)
  • Verified test plan from PR
Same party test Third party test
13268-First.Party.iFrame.mov
13268-THird.Party.iFrame.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

block window.ethereum completely in 3p iframes
4 participants