b425ed981e  WebGeolocationManager::resetPermissions has an inverted iterator check

The bug at a glance

An inverted end()-iterator check causes resetPermissions() to dereference a past-the-end iterator of m_pageSets whenever the queried registrable domain is absent – an out-of-bounds / invalid-memory read in the WebContent process. It is medium rather than high because the trigger is a domain-not-found lookup on a geolocation reset path (limited attacker control over the resulting memory contents) and the observable effect is dereferencing it->value on an invalid iterator, most plausibly a crash rather than a controllable primitive.

A textbook polarity inversion: the guard reads if (it != end()) return; – returning early exactly when the entry was found (making the intended reset a silent no-op) and falling through to use the iterator precisely when it points past the end. Every sibling method in the file uses the correct if (it == end()) return; form, which is what makes the bug both obvious in hindsight and easy to miss in review.

Root cause

OBSERVED: WebGeolocationManager::resetPermissions(const RegistrableDomain&) does auto it = m_pageSets.find(registrableDomain); then the pre-patch guard if (it != m_pageSets.end()) return;. HashMap::find returns end() when the key is absent.

OBSERVED (from the surrounding code): after the guard the function does for (auto& page : copyToVector(it->value.pageSet)), i.e. it dereferences it->value. With the inverted check, the early return fires when the domain IS present – so a legitimate resetPermissions for a known domain becomes a no-op and permissions are never reset.

OBSERVED/INFERRED: when the domain is NOT present, find() returned end() and the inverted check does not return, so the code proceeds to dereference the end() iterator (it->value) of the m_pageSets HashMap. Reading ->value through a past-the-end HashMap iterator is undefined behavior: it accesses memory at the bucket the end sentinel points to, which is an out-of-bounds/invalid read that typically manifests as a crash but is UB.

OBSERVED: the fix flips the comparison to if (it == m_pageSets.end()) return;, matching every other method in the file, so absent domains return early and only present entries are dereferenced.

Key code

The one-line polarity fix in resetPermissions (verbatim)

void WebGeolocationManager::resetPermissions(const WebCore::RegistrableDomain& registrableDomain)
{
    auto it = m_pageSets.find(registrableDomain);
    if (it == m_pageSets.end())
        return;

    for (auto& page : copyToVector(it->value.pageSet))

Patch walkthrough

• Source/WebKit/WebProcess/Geolocation/WebGeolocationManager.cpp — In resetPermissions(), the guard is changed from if (it != m_pageSets.end()) to if (it == m_pageSets.end()). This makes the early return fire when the domain is absent (the correct no-op) instead of when it is present, eliminating both the silent failure-to-reset for known domains and the past-the-end dereference of it->value for unknown domains.

Background

WebGeolocationManager — The WebProcess-side manager that tracks, per registrable domain, the set of pages using geolocation (m_pageSets, a HashMap keyed by RegistrableDomain). It coordinates permission state and high-accuracy requests between the web content and the UI process.

resetPermissions() — Clears cached geolocation permission state for a given registrable domain and notifies its pages. It is invoked in response to permission-reset events; the intended behavior is to iterate the domain’s pageSet only when the domain has a live entry.

HashMap end() iterator semantics — WTF::HashMap::find returns a past-the-end iterator (end()) when the key is not found. Dereferencing that iterator (it->value) is undefined behavior – it reads through a sentinel that does not point at a valid key/value pair, an out-of-bounds access.

Consistent guard idiom — The commit notes every other method in WebGeolocationManager.cpp uses if (it == end()) return; The inverted spelling in this one function is the anomaly; matching the established idiom is the fix.

Vulnerability window

1. Introduction — resetPermissions is written with an inverted end() check (if (it != end()) return;), diverging from the file’s other methods.
2. Latent dual bug — Known domains: reset silently no-ops (functional bug). Unknown domains: fall through to dereference the end() iterator (memory-safety bug).
3. Report — Filed as bug 317161 / rdar://179755180: ‘resetPermissions has an inverted iterator check.’
4. Fix — Flip the comparison to if (it == m_pageSets.end()) return; (315287@main), reviewed by Rupin Mittal.

Triggering

No test was added. Trigger (INFERRED from code): cause WebGeolocationManager::resetPermissions to be invoked with a RegistrableDomain that has no entry in m_pageSets (i.e. a domain that has not registered any geolocation-using page in this WebProcess). The find() returns end(), the inverted check fails to return, and the subsequent it->value dereference reads through the past-the-end iterator.

Exploitation

1. Reachability — Requires driving the geolocation permission-reset path for a domain absent from m_pageSets. INFERRED: control over which domains are registered vs. which is passed to resetPermissions is limited from ordinary web content.
2. Primitive — Dereferencing a HashMap end() iterator yields an invalid read of it->value.pageSet followed by copyToVector over it. Crash-only in practice; the attacker does not obviously control the contents read through the sentinel, so this is best characterized as a reliability/DoS and UB hazard rather than a shaped read/write primitive.
3. Honest assessment — No evidence in the patch of a write primitive or attacker-chosen address; treat as a memory-safety correctness fix whose worst realistic outcome is a WebProcess crash.

Detection & hunting

For defenders and SOC / detection engineers:

• ASan on absent-domain reset —
• Iterator-guard lint —

Audit directions

• Sibling methods in WebGeolocationManager.cpp —
• Other manager classes with per-domain HashMaps —
• Reset/teardown paths —