fix(privacy): Issues with content filtering in local frames on iOS

[StephenHeaps]

Resolves brave/brave-browser#40649

Submitter Checklist:

• I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
• 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 wiki
  • npm run presubmit wiki, 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:

• The associated issue milestone is set to the smallest version that the
  changes has landed on
• All relevant documentation has been updated, for instance:
  • https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • https://github.com/brave/brave-browser/wiki/Fingerprinting-Protections
  • https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • https://github.com/brave/brave-browser/wiki/Web-Compatibility-Exceptions-in-Brave
  • https://github.com/brave/brave-browser/wiki/QA-Guide
  • https://github.com/brave/brave-browser/wiki/P3A

Test Plan:

Add custom filter rules:

stephenheaps.github.io/$first-party
stephenheaps.github.io##should-be-hidden
stephenheaps.github.io##+js(set, sval, 1)

Cosmetic Filtering:

1. Visit http://stephenheaps.github.io/local-frames/test.html
2. Verify This should be hidden text is hidden / not displayed on the main frame or iframe
3. Verify Brave logo image is hidden / not displayed on the main frame or iframe

Request Blocking:

1. Visit http://stephenheaps.github.io/local-frames/test.html
2. Verify This should be blocked is not displayed on the main frame or iframe

Scriptlets:

1. Visit http://stephenheaps.github.io/local-frames/test.html
2. Verify First party body has value of 1
3. Verify First local frame body has a value of 1

[pes10k]

This all looks great to me. My only comment is that it looks like windowLocationHref isn't used anywhere (i checked bc I was curious why you needed both windowLocationHref and windowOrigin). Unless I'm missing something, maybe all these references / values can be deleted, just to keep things tidy.

Not a blocking issue though. This looks great. Thank you!

Also, just to check my understanding, the other local frame issues like scriptlets will be in a diff PR?

[pes10k]

lgtm. Added some non-blocking comments too

[StephenHeaps]

This all looks great to me. My only comment is that it looks like windowLocationHref isn't used anywhere (i checked bc I was curious why you needed both windowLocationHref and windowOrigin). Unless I'm missing something, maybe all these references / values can be deleted, just to keep things tidy.

Not a blocking issue though. This looks great. Thank you!

Also, just to check my understanding, the other local frame issues like scriptlets will be in a diff PR?

@pes10k Yes I can remove windowLocationHref as unused now. The local frame issues with request blocking, cosmetic filtering and scriptlets should be resolved with this PR.

[pes10k]

Gotcha. +1 for removing the unneeded windowLocationHref value to keep things need-and-tidy

If this PR will also cover the scriptlet case, then I suggest adding a test for that too, since that'll touch some different code paths too.

[Brandon-T]

Why do you need (${window.origin})? You can just use window.origin no? Likewise do we need empty back-ticks instead of ''?

[StephenHeaps]

This is just in the log to try and be a bit more helpful / descriptive when the request comes from an about:blank iframe it will report as Brave prevented frame displaying about:blank (brave.com) from loading a resource from brave.com/some-resource.js (if we only displayed window.origin it wouldn't be clear it came from the iframe as only brave.com would be listed).

I can replace the empty backticks with ''.

[pes10k]

Just to note that there will be many cases where window.location.href !== window.origin that are irrelevant to local frames. window.location.href is the full URL of the current frame, while window.origin is the security origin. For this github page, the former is https://github.com/brave/brave-core/pull/26622, and the latter is https://github.com/.

If this check is intended to just check "is this an about:blank" frame, you might want something like window.location.origin !== window.origin

[StephenHeaps]

Yeah the intention was to provide more info for all cases - not just about:blank - as the request(s) are now blocked against window.origin not window.location.href. I can just log window.origin here and leave out the window.location.href from the console info log if this is causing confusion, the intention was just to be a bit more descriptive (otherwise the log wouldn't indicate if the request was blocked on the main frame or from an iframe).

[stoletheminerals]

I haven't looked at the code yet, but I think websites can still bypass all the overrides we apply by doing this https://github.com/brave/internal/issues/914

[stoletheminerals]

I built this PR and tested my PoC from https://github.com/brave/internal/issues/914. It's still possible to bypass JS injects:

[brave-builds]

Released in v1.75.71