Skip to content

Commit

Permalink
Bug 1858143 - "has storage access" should only persist during navigat…
Browse files Browse the repository at this point in the history
…ions that do not change the iframe's window origin - r=anti-tracking-reviewers,smaug,pbz

Minor correction from https://phabricator.services.mozilla.com/D184821.
The definition of "same-origin" used in that patch was that the iframe's origin after navigation is the same as the triggering principal.
This was incorrect.
Instead, the origin of the iframe before navigation should be the same as after navigation, which is the frame's document principal at the time this is called.

Also, I found places where I missed adding the new fields to the loadinfo: LocationBase and nsFrameLoader.
And I added the redirect tainting check and a missing nullcheck before calling SetTriggeringWindowId in nsDocShell.

Differential Revision: https://phabricator.services.mozilla.com/D190577
  • Loading branch information
bvandersloot-mozilla committed Oct 26, 2023
1 parent a61ba91 commit ae1d974
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 21 deletions.
4 changes: 3 additions & 1 deletion docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10453,7 +10453,9 @@ nsresult nsDocShell::DoURILoad(nsDocShellLoadState* aLoadState,
if (context->HasValidTransientUserGestureActivation()) {
aLoadState->SetHasValidUserGestureActivation(true);
}
aLoadState->SetTriggeringWindowId(context->Id());
if (!aLoadState->TriggeringWindowId()) {
aLoadState->SetTriggeringWindowId(context->Id());
}
if (!aLoadState->TriggeringStorageAccess()) {
Document* contextDoc = context->GetExtantDoc();
if (contextDoc) {
Expand Down
3 changes: 3 additions & 0 deletions dom/base/LocationBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ already_AddRefed<nsDocShellLoadState> LocationBase::CheckURL(
loadState->SetHasValidUserGestureActivation(
doc->HasValidTransientUserGestureActivation());

loadState->SetTriggeringWindowId(doc->InnerWindowID());
loadState->SetTriggeringStorageAccess(doc->UsingStorageAccess());

return loadState.forget();
}

Expand Down
6 changes: 6 additions & 0 deletions dom/base/nsFrameLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,12 @@ nsresult nsFrameLoader::ReallyStartLoadingInternal() {

loadState->SetFirstParty(false);

Document* ownerDoc = mOwnerContent->OwnerDoc();
if (ownerDoc) {
loadState->SetTriggeringStorageAccess(ownerDoc->UsingStorageAccess());
loadState->SetTriggeringWindowId(ownerDoc->InnerWindowID());
}

// If we're loading the default about:blank document in a <browser> element,
// prevent the load from causing a process switch by explicitly overriding
// remote type selection.
Expand Down

This file was deleted.

51 changes: 34 additions & 17 deletions toolkit/components/antitracking/AntiTrackingUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "AntiTrackingUtils.h"

#include "AntiTrackingLog.h"
#include "HttpBaseChannel.h"
#include "mozilla/BasePrincipal.h"
#include "mozilla/Components.h"
#include "mozilla/dom/BrowsingContext.h"
Expand All @@ -28,6 +29,7 @@
#include "nsIURI.h"
#include "nsNetUtil.h"
#include "nsPIDOMWindow.h"
#include "nsQueryObject.h"
#include "nsRFPService.h"
#include "nsSandboxFlags.h"
#include "nsScriptSecurityManager.h"
Expand Down Expand Up @@ -546,31 +548,46 @@ AntiTrackingUtils::GetStoragePermissionStateInParent(nsIChannel* aChannel) {
return nsILoadInfo::HasStoragePermission;
}

WindowContext* wc = bc->GetCurrentWindowContext();
if (!wc) {
return nsILoadInfo::NoStoragePermission;
}
WindowGlobalParent* wgp = wc->Canonical();
if (!wgp) {
return nsILoadInfo::NoStoragePermission;
}
nsIPrincipal* framePrincipal = wgp->DocumentPrincipal();
if (!framePrincipal) {
return nsILoadInfo::NoStoragePermission;
}

if (policyType == ExtContentPolicy::TYPE_SUBDOCUMENT) {
// For loads of framed documents, we only use storage access
// if the load is the result of a same-origin, self-initiated
// navigation of the frame.
uint64_t targetWindowIdNoTop = bc->GetCurrentInnerWindowId();
uint64_t triggeringWindowId;
loadInfo->GetTriggeringWindowId(&triggeringWindowId);
bool triggeringStorageAccess;
loadInfo->GetTriggeringStorageAccess(&triggeringStorageAccess);
if (targetWindowIdNoTop == triggeringWindowId && triggeringStorageAccess &&
trackingPrincipal->Equals(loadInfo->TriggeringPrincipal())) {
return nsILoadInfo::HasStoragePermission;
}
} else if (!bc->IsTop()) {
// For subframe resources, check if the document has storage access
// and that the resource being loaded is same-site to the page.
WindowContext* wc = bc->GetCurrentWindowContext();
if (!wc) {
rv = loadInfo->GetTriggeringWindowId(&triggeringWindowId);
if (NS_WARN_IF(NS_FAILED(rv))) {
return nsILoadInfo::NoStoragePermission;
}
WindowGlobalParent* wgp = wc->Canonical();
if (!wgp) {
bool triggeringWindowHasStorageAccess;
rv =
loadInfo->GetTriggeringStorageAccess(&triggeringWindowHasStorageAccess);
if (NS_WARN_IF(NS_FAILED(rv))) {
return nsILoadInfo::NoStoragePermission;
}
nsIPrincipal* framePrincipal = wgp->DocumentPrincipal();
if (!framePrincipal) {
return nsILoadInfo::NoStoragePermission;
RefPtr<net::HttpBaseChannel> httpChannel = do_QueryObject(aChannel);

if (targetWindowIdNoTop == triggeringWindowId &&
triggeringWindowHasStorageAccess &&
trackingPrincipal->Equals(framePrincipal) && httpChannel &&
!httpChannel->HasRedirectTaintedOrigin()) {
return nsILoadInfo::HasStoragePermission;
}
} else if (!bc->IsTop()) {
// For subframe resources, check if the document has storage access
// and that the resource being loaded is same-site to the page.
bool isThirdParty = true;
nsresult rv = framePrincipal->IsThirdPartyURI(trackingURI, &isThirdParty);
if (NS_SUCCEEDED(rv) && wc->GetUsingStorageAccess() && !isThirdParty) {
Expand Down

0 comments on commit ae1d974

Please sign in to comment.