After some discussion offline, we're going to sanitize the macro keys and
values with EscapeQueryParamValue
<https://source.chromium.org/chromium/chromium/src/+/main:base/strings/escape.h;l=29?q=EscapeQueryParamValue%20-f:out>
so
that macro substitution always stays within the original query parameter
field. This should prevent XSS-y substitutions while keeping the API
surface exactly the same for regular use.
This fix will be implemented in M118, and we've decided that it need not
block launch in M117 due to the low severity (especially because the
existing reportEvent surface grants the buyer reporting worklet complete
control over the destination URL, so the ad creative is already known to
trust the buyer wrt where its reports get sent).
On Monday, August 28, 2023 at 11:55:39 AM UTC-4 Garrett Tanzer wrote:
> Hi Daniel,
>
> - There are a few relevant call sites in the overall reporting flow:
> - Declare allowlist of reporting destination origins
> - This happens in navigator.joinAdInterestGroup(), by an ad
> auction buyer
> - Declare macros (key:value correspondences)
> - This happens in the buyer reporting worklet, by the same ad
> auction buyer that declared the reporting destination origins
> - Perform report to custom url
> - This happens under the auction's winning ad creative's origin,
> which isn't necessarily the same as the ad auction buyer, but it is
> chosen
> by the ad auction buyer
> - Here is the sequence of events:
>
>
> 1.
> 1. The browser ingests and validates the allowlist when the
> interest group is declared.
> 2. The browser ingests the macro key:value mapping when the auction
> happens. The key/value strings have no structure that is validated. The
> browser adds "${" and "}" around the user-provided keys in the macro
> mapping.
> 3. In the ad that results from the auction:
> 1. The ad sends a URL to the browser, including macros like
> ${KEY}. The URL has to be a valid HTTPS url even with the macros
> unsubstituted. (See impl:
>
> https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_host_impl.cc;l=8644?q=content%2Fbrowser%2Frenderer_host%2Frender_frame_host_impl.cc%20-f:out
> )
> 2. The browser performs a simple string substitution to replace
> the keys with the values. The implementation is reused from
> navigator.deprecatedReplaceInURN, another part of Protected
> Audience. It
> doesn't substitute macros recursively, so you can't get an infinite
> loop.
> deprecatedReplaceInURN has apparently not been spec'd, which is was
> unable
> to quickly reuse that spec and left it as a TODO for now.
> 3. The origin of the resulting URL is checked against the
> allowlist. If it doesn't match any of them, no action is performed.
> If the
> URL is invalid, this will create an opaque origin and therefore
> always fail
> the check. (See impl:
>
> https://source.chromium.org/chromium/chromium/src/+/main:content/browser/fenced_frame/fenced_frame_reporter.cc;l=454?q=fenced_frame_reporter.cc%20-f:out)
>
> We may add an explicit URL valid/HTTPS check after substitution for
> neatness/to be robust to any future changes to the allowlist
> checking.
> 4. If the URL does pass the check, a GET fetch will be performed
> on it (the result is unused). There's no other parsing/usage of the
> URL
> other than performing a fetch.
>
> So in summary, we were not very concerned about XSS because the entity
> choosing the macro pairings is the one we want to protect, and the entity
> providing the base URLs is semi-trusted. The less trusted entity is the
> site of the destination URL, which just receives a GET and doesn't get to
> control anything. Since the URL has to be valid both before and after the
> macro substitution, this limits how wacky the substitutions can get even if
> you don't trust one of those entities. The weirdest stuff you could do
> would be like "FOO}&key=value1&${BAR" -> "key=value2", so that it becomes
> "${FOO}&key=value1&${BAR}" -> "key=value2". We can add a check to exclude
> $,{,} characters in the macro key so even this isn't possible.
>
> Hope this answers your questions,
> Garrett
>
> On Monday, August 28, 2023 at 9:35:34 AM UTC-4 Daniel Vogelheim wrote:
>
>> Hi Liam,
>>
>> This intent has come up in the OWP security triage, and I'm trying to
>> figure out whether there's XSS potential in the 3rd sub-feature, "Creative
>> macros in FFAR". This looks like a string-based pattern replacement where
>> the result string will then be parsed by the browser. Similar things have
>> lent themselves to XSS, e.g. when a string value contains meta characters
>> that code downstream will then parse in unexpected ways. Unfortunately, I
>> can't seem to find enough information about how exactly the replacement &
>> subsequent usage works to make up my mind whether there's a concern or not.
>> Could you help me out with a bit more information?
>>
>> What I've found is this:
>> - In "<dfn>send a beacon</dfn>": "TODO: Substitute macros from |macro
>> map| into |destination url|." (source
>> <https://github.com/WICG/fenced-frame/pull/113/files>)
>> - In "<dfn>asynchronously finish reporting</dfn>" I find where macro map
>> being set, but then it says: "TODO: Pass |macroMap| and
>> |allowedReportingOrigins| to [=Finalize a reporting destination=] when it
>> is updated to take the parameters." (source
>> <https://github.com/WICG/turtledove/pull/762/files>) I can't find that
>> usage.
>>
>> Questions I have:
>> - Am I reading the right docs? Where else should I look?
>> - Is this meant as a simple string-based substitution? Is there any
>> filtering of allowed characters, or so?
>> - What happens with the result values?
>> - Is the entity that sets the macro values always the same that has set
>> the patterns the values are being used for, or could those be different
>> entities?
>>
>> Thanks!
>>
>>
>> On Fri, Aug 25, 2023 at 9:34 PM 'Liam Brady' via blink-dev <
>> blin...@chromium.org> wrote:
>>
>>> Contact emails
>>>
>>> shiva...@chromium.org, d...@chromium.org, jka...@chromium.org,
>>> lbr...@google.com
>>> Explainer(s)
>>>
>>> Send Automatic Beacons Once
>>>
>>> https://github.com/WICG/turtledove/pull/718
>>>
>>> Serializable Fenced Frames Configs - Minor Change, No explainer
>>> available.
>>>
>>> Note: With this change, FencedFrameConfig objects will be serializable
>>> and can be sent through "postMessage()" and other similar calls.
>>>
>>> Creative Macros in Fenced Frames Ads Reporting (FFAR)
>>>
>>> https://github.com/WICG/turtledove/pull/763
>>>
>>> Spec(s)
>>>
>>> Send Automatic Beacons Once
>>>
>>> https://github.com/WICG/fenced-frame/pull/109
>>>
>>> Serializable Fenced Frames Configs
>>>
>>> https://github.com/WICG/fenced-frame/pull/111
>>>
>>> Creative Macros in Fenced Frames Ads Reporting (FFAR)
>>>
>>> Protected Audience: https://github.com/WICG/turtledove/pull/762/files
>>>
>>> Fenced Frames: https://github.com/WICG/fenced-frame/pull/113
>>>
>>>
>>>
>>> Summary
>>>
>>> We launched Fenced Frames as a part of Chrome 115. We would like to add
>>> the following three functionalities to Fenced Frames.
>>>
>>> 1. Send Automatic Beacons Once
>>>
>>> A common feature in ad frames is the "why this ad?" link. Since that
>>> link is separate from the ad itself, clicking "why this ad?", and its
>>> subsequent navigation, should be considered different from clicking
>>> directly on the advertisement itself. With our current automatic beacon
>>> design, however, once an automatic beacon is set (usually in the click
>>> handler for the ad link), that beacon will send out for any subsequent
>>> top-level navigations, including if "why this ad?" is clicked. This can
>>> result in erroneous impressions being sent out.
>>>
>>> As a solution, this feature introduces a new "once" member to the
>>> FenceEvent <https://wicg.github.io/fenced-frame/#dictdef-fenceevent>
>>> dictionary passed into
>>> "window.fence.setReportEventDataForAutomaticBeacons()". If set to true, the
>>> saved automatic beacon data will be cleared out after the next beacon is
>>> sent, ensuring that automatic beacons with that data are only sent once.
>>> This means that further clicks to non-ad parts of the frame that result in
>>> top-level navigations will not send out erroneous beacons.
>>>
>>> (This feature already shipped in M116. That was our mistake. At the
>>> time, we thought we would only need a PSA for it, and shipped it without it
>>> being behind a flag. We apologize for the mistake.)
>>>
>>> 2. Serializable FencedFramesConfigs
>>>
>>> With this change, FencedFrameConfig objects will be serializable and can
>>> be sent through "postMessage()" and other similar calls. Serialization
>>> allows for a case where the frame that runs an ad auction is not the same
>>> frame that ends up embedding the winning ad in a fenced frame.
>>> FencedFrameConfigs cannot be serialized to storage, nor can they be sent in
>>> a message that crosses a fenced frame boundary. A FencedFrameConfig object
>>> is only valid in the traversable navigable
>>> <https://wicg.github.io/fenced-frame/#traversable-navigables> it was
>>> originally created in, and, if sent outside to a different context, will
>>> not be able to navigate, since the new traversable navigable
>>> <https://wicg.github.io/fenced-frame/#traversable-navigables>'s fenced
>>> frame config mapping
>>> <https://wicg.github.io/fenced-frame/#traversable-navigable-fenced-frame-config-mapping>
>>>
>>> will not contain the internal config needed to do the navigation.
>>>
>>> 3. Creative macros in Fenced Frames Ads Reporting (FFAR)
>>>
>>> This feature extends the Fenced Frame Ads Reporting (FFAR) API to
>>> support macro substitution in reporting URLs and allows reports to be sent
>>> to up to ten other origins that have enrolled with the Privacy Sandbox and
>>> allow-listed by the DSP. Use case: In online ad auctions for ad space,
>>> advertisers buying through DSPs in several situations use other adtech
>>> providers to monitor performance and keep track of how their advertising
>>> dollars are spent. (issue link
>>> <https://github.com/WICG/turtledove/issues/477>)
>>>
>>> Blink component
>>>
>>> Blink>FencedFrames
>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EFencedFrames>
>>>
>>> TAG reviews and status
>>>
>>> Fenced frames existing TAG review appended with these spec changes
>>> https://github.com/w3ctag/design-reviews/issues/838#issuecomment-1693631006
>>>
>>> Link to Origin Trial feedback summary
>>>
>>> No Origin Trial performed
>>>
>>> Is this feature supported on all six Blink platforms (Windows, Mac,
>>> Linux, Chrome OS, Android, and Android WebView)?
>>>
>>> Supported on all the above platforms except Android WebView.
>>>
>>> Debuggability
>>>
>>> Additional debugging capabilities are not necessary for these feature
>>> changes.
>>>
>>> Risks
>>>
>>> Compatibility
>>>
>>> There are no compatibility risks, as described below:
>>>
>>> 1. Send Automatic Beacons Once: This is backward compatible with the
>>> existing API since the default value of “once” is false which is the same
>>> behavior as the previous behavior.
>>>
>>> 2. Serializable FencedFramesConfigs: This is added functionality and
>>> backward compatible with the existing FencedFramesConfig.
>>>
>>> 3. Creative macros in Fenced Frames Ads Reporting (FFAR): This is
>>> adding a new API and a backward compatible change to reportEvent.
>>>
>>> Interoperability
>>>
>>> there are no interoperability risks as no other browsers have decided to
>>> implement these features yet.
>>>
>>> Is this feature fully tested by web-platform-tests
>>> <https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_platform_tests.md>?
>>>
>>> Link to test suite results from wpt.fyi.
>>>
>>> Yes
>>>
>>> Tests:
>>> https://github.com/web-platform-tests/wpt/tree/master/fenced-frame
>>>
>>> Results:
>>> https://wpt.fyi/results/fenced-frame?label=experimental&label=master&aligned
>>>
>>> Specifically, these features correspond to the following tests:
>>>
>>> Send Automatic Beacons Once:
>>>
>>> -
>>>
>>> automatic-beacon-two-events-clear.https.html (test
>>>
>>> <https://github.com/web-platform-tests/wpt/blob/master/fenced-frame/automatic-beacon-two-events-clear.https.html>)
>>>
>>> (result
>>>
>>> <https://wpt.fyi/results/fenced-frame/automatic-beacon-two-events-clear.https.html?label=experimental&label=master&aligned>
>>> )
>>> -
>>>
>>> automatic-beacon-two-events-persist.https.html (test
>>>
>>> <https://github.com/web-platform-tests/wpt/blob/master/fenced-frame/automatic-beacon-two-events-persist.https.html>)
>>>
>>> (result
>>>
>>> <https://wpt.fyi/results/fenced-frame/automatic-beacon-two-events-persist.https.html?label=experimental&label=master&aligned>
>>> )
>>>
>>> Serializable FencedFrameConfigs:
>>>
>>> -
>>>
>>> deep-copy-config.https.html (test
>>>
>>> <https://github.com/web-platform-tests/wpt/blob/master/fenced-frame/deep-copy-config.https.html>)
>>>
>>> (result
>>>
>>> <https://wpt.fyi/results/fenced-frame/deep-copy-config.https.html?label=experimental&label=master&aligned>
>>> )
>>>
>>> Creative macros in Fenced Frames Ads Reporting (FFAR):
>>>
>>> -
>>>
>>> fence-report-event-destination-url.https.html (test
>>>
>>> <https://github.com/web-platform-tests/wpt/blob/master/fenced-frame/fence-report-event-destination-url.https.html>)
>>>
>>> (result
>>>
>>> <https://wpt.fyi/results/fenced-frame/fence-report-event-destination-url.https.html?label=experimental&label=master&aligned>
>>> )
>>>
>>>
>>> Anticipated spec changes
>>>
>>> None
>>>
>>> Link to entry on the Chrome Platform Status
>>>
>>> https://chromestatus.com/feature/5103970808233984
>>>
>>> Links to previous Intent discussions
>>>
>>> Intent to prototype:
>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/Ko9UXQYPgUE/m/URRsB-qvAAAJ
>>>
>>>
>>> Intent to experiment:
>>>
>>>
>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/y6G3cvKXjlg/m/Lcpmpi_LAgAJ
>>>
>>>
>>> Intent to extend origin trial:
>>>
>>>
>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/SD8Ot2gpz4g/m/A9uA-_cGAwAJ
>>>
>>>
>>>
>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/gpmaOi3of_w/m/SyMclFhMAAAJ
>>>
>>>
>>>
>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/CBrV-2DrYFI/m/RTojC6kHAgAJ
>>>
>>>
>>> Intent to ship:
>>>
>>> https://groups.google.com/a/chromium.org/g/blink-dev/c/tpw8wW0VenQ/m/mePLTiHlDQAJ
>>>
>>>
>>> --
>>> You received this message because you are subscribed to the Google
>>> Groups "blink-dev" group.
>>> To unsubscribe from this group and stop receiving emails from it, send
>>> an email to blink-dev+...@chromium.org.
>>> To view this discussion on the web visit
>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/62771371-5dbb-4d02-a232-a99ded5b293fn%40chromium.org
>>>
>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/62771371-5dbb-4d02-a232-a99ded5b293fn%40chromium.org?utm_medium=email&utm_source=footer>
>>> .
>>>
>>
--
You received this message because you are subscribed to the Google Groups
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/2b14ffdf-1fdb-4fa7-9054-65356c007b50n%40chromium.org.
