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

Blocked requests inside nested "about:blank" iFrames are not counted properly #1019

Open
alisha opened this issue Oct 15, 2024 · 1 comment
Labels

Comments

@alisha
Copy link

alisha commented Oct 15, 2024

We found a minor bug in the DuckDuckGo macOS browser (version 1.102.0) where some requests are blocked, but are not counted in DuckDuckGo's count of how many trackers have been blocked. This occurs when requests are made inside nested "about:blank" iFrames.

Reproducing

  1. Visit a website; we test on rocketnews24.com
  2. Open a new tab, and observe how many things are blocked (see screenshot below)

DDG Trackers Blocked - Original

  1. Create a pair of nested frames with source "about:blank." We do this by entering the following code in the JavaScript console.
const [pframe, cframe] = (_ => {const parentFrame = document.createElement('iframe');const childFrame = document.createElement('iframe');parentFrame.name = "/parent-frame";parentFrame.id = "parent-frame";childFrame.name = "/child-frame";childFrame.id = "child-frame";document.body.appendChild(parentFrame);parentFrame.contentDocument.body.appendChild(childFrame);return  [parentFrame, childFrame];})()
  1. Add a script that DDG will block to the parent frame. We do this by entering the following code in the JavaScript console.
((aSrc, frameElm) => {const scriptElm = document.createElement('script'); scriptElm.src = aSrc; frameElm.contentDocument.body.appendChild(scriptElm);})('https://rtb.openx.net/openrtbb/prebidjs', pframe);
  1. Open a new tab, and observe how many things are blocked (see screenshot below). We find that the count increases by one, which is the expected behavior.

DDG Trackers Blocked - Parent Frame

  1. Add the same script to the child frame. We do this by entering the following code in the JavaScript console.
((aSrc, frameElm) => {const scriptElm = document.createElement('script'); scriptElm.src = aSrc; frameElm.contentDocument.body.appendChild(scriptElm);})('https://rtb.openx.net/openrtbb/prebidjs', cframe);
  1. Open a new tab, and observe how many things are blocked (see screenshot below). The count has not increased, but we expect the count to increase.

DDG Trackers Blocked - Child Frame

Impact

Users will not understand the privacy-harming behaviors of the websites they visit, or the privacy benefits they receive through using DuckDuckGo.

Fix

We will submit a PR shortly that fixes this issue by modifying Sources/BrowserServicesKit/ContentBlocking/UserScripts/contentblockerrules.js

@jonathanKingston
Copy link
Collaborator

These code snippets were really great in illustrating the problem, thank you for that!

brindy pushed a commit that referenced this issue Oct 16, 2024
…js (#1021)

<!--
Note: This checklist is a reminder of our shared engineering
expectations.
-->

Please review the release process for BrowserServicesKit
[here](https://app.asana.com/0/1200194497630846/1200837094583426).

**Required**:

Task/Issue URL: #1019 and
https://app.asana.com/0/1200437802575119/1208550540369943/f
iOS PR: TODO
macOS PR: TODO
What kind of version bump will this require?: Minor

**Optional**:

Tech Design URL:
CC:

**Description**:

This PR changes the tab calculation for the page, currently it doesn't
account for grand child frames at all.
This code has mostly been cribbed from:
https://github.com/duckduckgo/content-scope-scripts/blob/655d12d9e77f93cc36875142aa2ff3e06c52608d/injected/src/utils.js#L135-L157

Full disclosure: I tested this manually in the console rather than doing
a full build, we should verify before releasing.

Thanks to @alisha for spotting this problem in #1019, producing a well
written explanation and PR!

**Steps to test this PR**:
For testing the blocking follow the steps in:
#1019 using the
console.

For surrogates the same should work for a tracker on the surrogate list:
- replacing https://rtb.openx.net/openrtbb/prebidjs with
https://google-analytics.com/gtm/js
- validating that frame has the script executed: cframe.contentWindow.ga
should be defined

<!--
Before submitting a PR, please ensure you have tested the combinations
you expect the reviewer to test, then delete configurations you *know*
do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
-->

**OS Testing**:

* [ ] iOS 14
* [ ] iOS 15
* [ ] iOS 16
* [ ] macOS 10.15
* [ ] macOS 11
* [ ] macOS 12

---
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
@github-actions github-actions bot added the stale label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants