Skip to content

Commit

Permalink
Bug 1810619 - Part 1: Be more precise in named lookup code, r=smaug,g…
Browse files Browse the repository at this point in the history 
…eckoview-reviewers,m_kato

This makes various changes to the named lookup/navigation code to make
them more precise, and avoid issues which could happen if a window is
closed while script is still executing.

This also should improve handling for inactive windows in some cases, by
more frequently working off of the WindowContext tree rather than the
BrowsingContext tree.

As part of these changes, some behaviour was changed around e.g. the
file URI exception to avoid the deprecated nsIPrincipal::GetURI method.
I don't believe the behaviour should have changed in a meaningful way.

Differential Revision: https://phabricator.services.mozilla.com/D171755
  • Loading branch information
@mystor
mystor committed Mar 8, 2023
1 parent 8a81a95 commit eece505
Show file tree
Hide file tree
Showing 17 changed files with 301 additions and 328 deletions.
162 changes: 15 additions & 147 deletions docshell/base/BrowsingContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1194,104 +1194,15 @@ void BrowsingContext::GetAllBrowsingContextsInSubtree(
});
}

// FindWithName follows the rules for choosing a browsing context,
// with the exception of sandboxing for iframes. The implementation
// for arbitrarily choosing between two browsing contexts with the
// same name is as follows:
//
// 1) The start browsing context, i.e. 'this'
// 2) Descendants in insertion order
// 3) The parent
// 4) Siblings and their children, both in insertion order
// 5) After this we iteratively follow the parent chain, repeating 3
// and 4 until
// 6) If there is no parent, consider all other top level browsing
// contexts and their children, both in insertion order
//
// See
// https://html.spec.whatwg.org/multipage/browsers.html#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name
BrowsingContext* BrowsingContext::FindWithName(
const nsAString& aName, bool aUseEntryGlobalForAccessCheck) {
RefPtr<BrowsingContext> requestingContext = this;
if (aUseEntryGlobalForAccessCheck) {
if (nsGlobalWindowInner* caller = nsContentUtils::EntryInnerWindow()) {
if (caller->GetBrowsingContextGroup() == Group()) {
requestingContext = caller->GetBrowsingContext();
} else {
MOZ_RELEASE_ASSERT(caller->GetPrincipal()->IsSystemPrincipal(),
"caller must be either same-group or system");
}
}
}
MOZ_ASSERT(requestingContext, "must have a requestingContext");

BrowsingContext* found = nullptr;
if (aName.IsEmpty()) {
// You can't find a browsing context with an empty name.
found = nullptr;
} else if (aName.LowerCaseEqualsLiteral("_blank")) {
// Just return null. Caller must handle creating a new window with
// a blank name.
found = nullptr;
} else if (nsContentUtils::IsSpecialName(aName)) {
found = FindWithSpecialName(aName, *requestingContext);
} else if (BrowsingContext* child =
FindWithNameInSubtree(aName, *requestingContext)) {
found = child;
} else {
BrowsingContext* current = this;

do {
Span<RefPtr<BrowsingContext>> siblings;
BrowsingContext* parent = current->GetParent();

if (!parent) {
// We've reached the root of the tree, consider browsing
// contexts in the same browsing context group.
siblings = mGroup->Toplevels();
} else if (parent->NameEquals(aName) &&
requestingContext->CanAccess(parent) &&
parent->IsTargetable()) {
found = parent;
break;
} else {
siblings = parent->NonSyntheticChildren();
}

for (BrowsingContext* sibling : siblings) {
if (sibling == current) {
continue;
}

if (BrowsingContext* relative =
sibling->FindWithNameInSubtree(aName, *requestingContext)) {
found = relative;
// Breaks the outer loop
parent = nullptr;
break;
}
}

current = parent;
} while (current);
}

// Helpers should perform access control checks, which means that we
// only need to assert that we can access found.
MOZ_DIAGNOSTIC_ASSERT(!found || requestingContext->CanAccess(found));

return found;
}

BrowsingContext* BrowsingContext::FindChildWithName(
const nsAString& aName, BrowsingContext& aRequestingContext) {
const nsAString& aName, WindowGlobalChild& aRequestingWindow) {
if (aName.IsEmpty()) {
// You can't find a browsing context with the empty name.
return nullptr;
}

for (BrowsingContext* child : NonSyntheticChildren()) {
if (child->NameEquals(aName) && aRequestingContext.CanAccess(child) &&
if (child->NameEquals(aName) && aRequestingWindow.CanNavigate(child) &&
child->IsTargetable()) {
return child;
}
Expand All @@ -1301,7 +1212,7 @@ BrowsingContext* BrowsingContext::FindChildWithName(
}

BrowsingContext* BrowsingContext::FindWithSpecialName(
const nsAString& aName, BrowsingContext& aRequestingContext) {
const nsAString& aName, WindowGlobalChild& aRequestingWindow) {
// TODO(farre): Neither BrowsingContext nor nsDocShell checks if the
// browsing context pointed to by a special name is active. Should
// it be? See Bug 1527913.
Expand All @@ -1311,81 +1222,40 @@ BrowsingContext* BrowsingContext::FindWithSpecialName(

if (aName.LowerCaseEqualsLiteral("_parent")) {
if (BrowsingContext* parent = GetParent()) {
return aRequestingContext.CanAccess(parent) ? parent : nullptr;
return aRequestingWindow.CanNavigate(parent) ? parent : nullptr;
}
return this;
}

if (aName.LowerCaseEqualsLiteral("_top")) {
BrowsingContext* top = Top();

return aRequestingContext.CanAccess(top) ? top : nullptr;
return aRequestingWindow.CanNavigate(top) ? top : nullptr;
}

return nullptr;
}

BrowsingContext* BrowsingContext::FindWithNameInSubtree(
const nsAString& aName, BrowsingContext& aRequestingContext) {
const nsAString& aName, WindowGlobalChild* aRequestingWindow) {
MOZ_DIAGNOSTIC_ASSERT(!aName.IsEmpty());

if (NameEquals(aName) && aRequestingContext.CanAccess(this) &&
if (NameEquals(aName) &&
(!aRequestingWindow || aRequestingWindow->CanNavigate(this)) &&
IsTargetable()) {
return this;
}

for (BrowsingContext* child : NonSyntheticChildren()) {
if (BrowsingContext* found =
child->FindWithNameInSubtree(aName, aRequestingContext)) {
child->FindWithNameInSubtree(aName, aRequestingWindow)) {
return found;
}
}

return nullptr;
}

// For historical context, see:
//
// Bug 13871: Prevent frameset spoofing
// Bug 103638: Targets with same name in different windows open in wrong
// window with javascript
// Bug 408052: Adopt "ancestor" frame navigation policy
// Bug 1570207: Refactor logic to rely on BrowsingContextGroups to enforce
// origin attribute isolation.
bool BrowsingContext::CanAccess(BrowsingContext* aTarget,
bool aConsiderOpener) {
MOZ_ASSERT(
mDocShell,
"CanAccess() may only be called in the process of the accessing window");
MOZ_ASSERT(aTarget, "Must have a target");

MOZ_DIAGNOSTIC_ASSERT(
Group() == aTarget->Group(),
"A BrowsingContext should never see a context from a different group");

// A frame can navigate itself and its own root.
if (aTarget == this || aTarget == Top()) {
return true;
}

// A frame can navigate any frame with a same-origin ancestor.
for (BrowsingContext* bc = aTarget; bc; bc = bc->GetParent()) {
if (bc->mDocShell && nsDocShell::ValidateOrigin(this, bc)) {
return true;
}
}

// If the target is a top-level document, a frame can navigate it if it can
// navigate its opener.
if (aConsiderOpener && !aTarget->GetParent()) {
if (RefPtr<BrowsingContext> opener = aTarget->GetOpener()) {
return CanAccess(opener, false);
}
}

return false;
}

bool BrowsingContext::IsSandboxedFrom(BrowsingContext* aTarget) {
// If no target then not sandboxed.
if (!aTarget) {
Expand Down Expand Up @@ -2070,13 +1940,12 @@ nsresult BrowsingContext::LoadURI(nsDocShellLoadState* aLoadState,

MOZ_DIAGNOSTIC_ASSERT(!sourceBC || sourceBC->Group() == Group());
if (sourceBC && sourceBC->IsInProcess()) {
if (!sourceBC->CanAccess(this)) {
return NS_ERROR_DOM_PROP_ACCESS_DENIED;
}

nsCOMPtr<nsPIDOMWindowOuter> win(sourceBC->GetDOMWindow());
if (WindowGlobalChild* wgc =
win->GetCurrentInnerWindow()->GetWindowGlobalChild()) {
if (!wgc->CanNavigate(this)) {
return NS_ERROR_DOM_PROP_ACCESS_DENIED;
}
wgc->SendLoadURI(this, aLoadState, aSetNavigating);
}
} else if (XRE_IsParentProcess()) {
Expand Down Expand Up @@ -2175,16 +2044,15 @@ nsresult BrowsingContext::InternalLoad(nsDocShellLoadState* aLoadState) {
MOZ_DIAGNOSTIC_ASSERT(sourceBC);
MOZ_DIAGNOSTIC_ASSERT(sourceBC->Group() == Group());

if (!sourceBC->CanAccess(this)) {
return NS_ERROR_DOM_PROP_ACCESS_DENIED;
}

nsCOMPtr<nsPIDOMWindowOuter> win(sourceBC->GetDOMWindow());
WindowGlobalChild* wgc =
win->GetCurrentInnerWindow()->GetWindowGlobalChild();
if (!wgc || !wgc->CanSend()) {
return NS_ERROR_FAILURE;
}
if (!wgc->CanNavigate(this)) {
return NS_ERROR_DOM_PROP_ACCESS_DENIED;
}

MOZ_ALWAYS_SUCCEEDS(
SetCurrentLoadIdentifier(Some(aLoadState->GetLoadIdentifier())));
Expand Down
37 changes: 12 additions & 25 deletions docshell/base/BrowsingContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class SessionHistoryInfo;
class SessionStorageManager;
class StructuredCloneHolder;
class WindowContext;
class WindowGlobalChild;
struct WindowPostMessageOptions;
class WindowProxyHolder;

Expand Down Expand Up @@ -659,32 +660,26 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
}
}

// Using the rules for choosing a browsing context we try to find
// the browsing context with the given name in the set of
// transitively reachable browsing contexts. Performs access control
// checks with regard to this.
// See
// https://html.spec.whatwg.org/multipage/browsers.html#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name.
//
// BrowsingContext::FindWithName(const nsAString&) is equivalent to
// calling nsIDocShellTreeItem::FindItemWithName(aName, nullptr,
// nullptr, false, <return value>).
BrowsingContext* FindWithName(const nsAString& aName,
bool aUseEntryGlobalForAccessCheck = true);

// Find a browsing context in this context's list of
// children. Doesn't consider the special names, '_self', '_parent',
// '_top', or '_blank'. Performs access control checks with regard to
// 'this'.
BrowsingContext* FindChildWithName(const nsAString& aName,
BrowsingContext& aRequestingContext);
WindowGlobalChild& aRequestingWindow);

// Find a browsing context in the subtree rooted at 'this' Doesn't
// consider the special names, '_self', '_parent', '_top', or
// '_blank'. Performs access control checks with regard to
// 'aRequestingContext'.
// '_blank'.
//
// If passed, performs access control checks with regard to
// 'aRequestingContext', otherwise performs no access checks.
BrowsingContext* FindWithNameInSubtree(const nsAString& aName,
BrowsingContext& aRequestingContext);
WindowGlobalChild* aRequestingWindow);

// Find the special browsing context if aName is '_self', '_parent',
// '_top', but not '_blank'. The latter is handled in FindWithName
BrowsingContext* FindWithSpecialName(const nsAString& aName,
WindowGlobalChild& aRequestingWindow);

nsISupports* GetParentObject() const;
JSObject* WrapObject(JSContext* aCx,
Expand Down Expand Up @@ -791,9 +786,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
BrowsingContextGroup* aGroup,
ContentParent* aOriginProcess);

// Performs access control to check that 'this' can access 'aTarget'.
bool CanAccess(BrowsingContext* aTarget, bool aConsiderOpener = true);

bool IsSandboxedFrom(BrowsingContext* aTarget);

// The runnable will be called once there is idle time, or the top level
Expand Down Expand Up @@ -964,11 +956,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
// parent WC changes.
void RecomputeCanExecuteScripts();

// Find the special browsing context if aName is '_self', '_parent',
// '_top', but not '_blank'. The latter is handled in FindWithName
BrowsingContext* FindWithSpecialName(const nsAString& aName,
BrowsingContext& aRequestingContext);

// Is it early enough in the BrowsingContext's lifecycle that it is still
// OK to set OriginAttributes?
bool CanSetOriginAttributes();
Expand Down
61 changes: 5 additions & 56 deletions docshell/base/nsDocShell.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1438,61 +1438,6 @@ nsDOMNavigationTiming* nsDocShell::GetNavigationTiming() const {
return mTiming;
}

//
// Bug 13871: Prevent frameset spoofing
//
// This routine answers: 'Is origin's document from same domain as
// target's document?'
//
// file: uris are considered the same domain for the purpose of
// frame navigation regardless of script accessibility (bug 420425)
//
/* static */
bool nsDocShell::ValidateOrigin(BrowsingContext* aOrigin,
BrowsingContext* aTarget) {
nsIDocShell* originDocShell = aOrigin->GetDocShell();
MOZ_ASSERT(originDocShell, "originDocShell must not be null");
Document* originDocument = originDocShell->GetDocument();
NS_ENSURE_TRUE(originDocument, false);

nsIDocShell* targetDocShell = aTarget->GetDocShell();
MOZ_ASSERT(targetDocShell, "targetDocShell must not be null");
Document* targetDocument = targetDocShell->GetDocument();
NS_ENSURE_TRUE(targetDocument, false);

bool equal;
nsresult rv = originDocument->NodePrincipal()->Equals(
targetDocument->NodePrincipal(), &equal);
if (NS_SUCCEEDED(rv) && equal) {
return true;
}
// Not strictly equal, special case if both are file: uris
nsCOMPtr<nsIURI> originURI;
nsCOMPtr<nsIURI> targetURI;
nsCOMPtr<nsIURI> innerOriginURI;
nsCOMPtr<nsIURI> innerTargetURI;

// Casting to BasePrincipal, as we can't get InnerMost URI otherwise
auto* originDocumentBasePrincipal =
BasePrincipal::Cast(originDocument->NodePrincipal());

rv = originDocumentBasePrincipal->GetURI(getter_AddRefs(originURI));
if (NS_SUCCEEDED(rv) && originURI) {
innerOriginURI = NS_GetInnermostURI(originURI);
}

auto* targetDocumentBasePrincipal =
BasePrincipal::Cast(targetDocument->NodePrincipal());

rv = targetDocumentBasePrincipal->GetURI(getter_AddRefs(targetURI));
if (NS_SUCCEEDED(rv) && targetURI) {
innerTargetURI = NS_GetInnermostURI(targetURI);
}

return innerOriginURI && innerTargetURI && SchemeIsFile(innerOriginURI) &&
SchemeIsFile(innerTargetURI);
}

nsPresContext* nsDocShell::GetEldestPresContext() {
nsIContentViewer* viewer = mContentViewer;
while (viewer) {
Expand Down Expand Up @@ -8499,7 +8444,11 @@ nsresult nsDocShell::PerformRetargeting(nsDocShellLoadState* aLoadState) {
aLoadState->Target().LowerCaseEqualsLiteral("_self") ||
aLoadState->Target().LowerCaseEqualsLiteral("_parent") ||
aLoadState->Target().LowerCaseEqualsLiteral("_top")) {
targetContext = mBrowsingContext->FindWithName(
Document* document = GetDocument();
NS_ENSURE_TRUE(document, NS_ERROR_FAILURE);
WindowGlobalChild* wgc = document->GetWindowGlobalChild();
NS_ENSURE_TRUE(wgc, NS_ERROR_FAILURE);
targetContext = wgc->FindBrowsingContextWithName(
aLoadState->Target(), /* aUseEntryGlobalForAccessCheck */ false);
}

Expand Down
Loading

0 comments on commit eece505

Please sign in to comment.