2032002 - Session history IPC handlers missing ownership check — compromised renderer modifies history entries of any origin

Status: RESOLVED INVALID

(Core :: DOM: Navigation, defect)

Description

[pakhunov.anton.n]

Several child-to-parent session history handlers in ContentParent.cpp accept a BrowsingContext argument from the content process without verifying the sender owns that entry. A compromised renderer in a BrowsingContextGroup can modify history metadata for any other origin's tabs/frames.

From PContent.ipdl (all in the parent: section, child-to-parent):

async SessionHistoryEntryTitle(MaybeDiscardedBrowsingContext aContext, nsString aTitle);
async SessionHistoryEntryScrollPosition(MaybeDiscardedBrowsingContext aContext, int32_t aX, int32_t aY);
async SessionHistoryEntryScrollRestorationIsManual(MaybeDiscardedBrowsingContext aContext, bool aIsManual);
async SessionHistoryEntryCacheKey(MaybeDiscardedBrowsingContext aContext, uint32_t aCacheKey);
async SessionHistoryEntryWireframe(MaybeDiscardedBrowsingContext aContext, Wireframe aWireframe);
async SessionHistoryEntryStoreWindowNameInContiguousEntries(MaybeDiscardedBrowsingContext aContext, nsString aName);
async SynchronizeNavigationAPIState(MaybeDiscardedBrowsingContext aContext, ...);
async RemoveFromBFCache(MaybeDiscardedBrowsingContext aContext);

The handlers in ContentParent.cpp look up the browsing context by ID and apply the supplied value directly, without checking IsOwnedByProcess or that aContext->Canonical()->GetContentParent() == this.

Most impactful: SessionHistoryEntryTitle -- tab title and browser history display reflect session history titles. A compromised evil.com renderer can set the title of bank.com's history entry to phishing text while bank.com is open in another tab.

Also: RemoveFromBFCache forces full reload for any origin's page; SynchronizeNavigationAPIState manipulates other origins' navigation API state; SessionHistoryEntryScrollPosition changes scroll position on back navigation.

Entry IDs are broadcast to BrowsingContextGroup members during navigation, so a compromised renderer in the same BCG (e.g. evil.com opened by bank.com) knows the IDs.

Attack:
Compromised evil.com renderer learns bank.com's BrowsingContext ID. Sends:
SessionHistoryEntryTitle(bank_com_bc_id, "Your session expired -- log in again")

The title change takes effect immediately in the tab bar and history panel. No JS execution in bank.com's process required.

Fix:
Add an ownership check in each handler before applying changes:

// Pattern to add at the top of each handler:
BrowsingContext* bc = aContext.GetMaybeDiscarded();
if (!bc || bc->Canonical()->GetContentParent() != this) {
return IPC_FAIL(this, "SessionHistoryEntry: sender does not own this BrowsingContext");
}

The CheckOnlyOwningProcessCanSet pattern from WindowContext.h is the right model.

Version: mozilla-central HEAD. All platforms.

Comment 1

[pakhunov.anton.n]

Attachment: Full bug report with code analysis

Comment 2

[Andrew McCreight [:mccr8]]

Please don't assign security ratings to your bugs.

Comment 3

[pakhunov.anton.n]

(In reply to Andrew McCreight [:mccr8] from comment #2)

Please don't assign security ratings to your bugs.

Ok, sorry

Comment 4

[Andrew McCreight [:mccr8]]

Checking was added for BCs in bug 2008912.