2034558 - nsHttpChannel::ParseDictionary leaks UrlPatternGlue (and rooted SpiderMonkey RegExp objects) on every Use-As-Dictionary response

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P2)

Description

[ukyo]

Attachment: patch.diff

Steps to reproduce:

Tested on Firefox Nightly 152.0a1 (BuildID 20260423082823, SourceStamp 2a5398c41a2cbc8221cfebda2e46759afd1fa482), macOS 26.4.1 arm64. Single-machine testing only; PoC
reproducer and user.js attached.

1. Launch Firefox Nightly with a fresh profile (--profile <empty-dir> --no-remote --new-instance). network.http.dictionaries.enable defaults to true on Nightly.

2. Run an HTTPS server with a locally-trusted cert (I used mkcert + certutil -A to import the mkcert root CA into the profile's cert9.db). Self-signed certs accepted
   through the Firefox exception UI are NOT cached, so the leak does not reproduce without a trusted chain. For every /dict/<unique> URL return:

   Content-Type: text/plain; charset=utf-8
   Cache-Control: public, max-age=3600
   Use-As-Dictionary: match="/*", id="leak-<counter>", type=raw

   plus an 8 KiB body.

3. From a page on that origin, issue 10000 fetches to distinct URLs:

   for (let i = 0; i < 10000; i++) { await fetch(/dict/${Date.now()}-${i}); }

4. Open about:memory in another tab, click Minimize memory usage, then Measure and save. Do this before step 3 and again after step 3. Diff the two reports.

Actual results:

The RegExp gc-heap counter in the XPConnect junk compartment grows by ~448 B per Use-As-Dictionary response and is NOT reclaimed by Minimize memory usage (full GC/CC).
Measurement on my machine, 10000 triggers:

path	before	after	delta
explicit/js-non-window/zones/zone(…)/realm(moz-nullprincipal:…, XPConnect Junk Compartment)/classes/class(RegExp)/objects/gc-heap	0	4,480,448	+4,480,448 (~10000
objects × ~448 B)
explicit/network/cache2/index	874,816	1,748,096	+873,280
heap-allocated (Main Process)	312,988,752	544,440,688	+231,451,936

The +4.48 MB rooted RegExp gc-heap persisting across full GC matches 10000 rooted objects — one per trigger — proving the bug. (The +231 MB total heap delta is dominated by
rendering about:cache?storage=disk in another tab to verify the 10000 cache entries landed; the cleanly leak-attributable part is the +4.48 MB pinned GC heap plus ~1–2 KiB
native per trigger.)

Root cause

In netwerk/protocol/http/nsHttpChannel.cpp (current Nightly):

bool nsHttpChannel::ParseDictionary(nsICacheEntry* aEntry,   
                                    nsHttpResponseHead* aResponseHead,
                                    bool aModified) {                                                                                                                        
  if (NS_SUCCEEDED(aResponseHead->GetHeader(nsHttp::Use_As_Dictionary, val))) {
    …                                                                                                                                                                        
    UrlPatternGlue pattern;                                  
    UrlPatternOptions options{};                                                                                                                                             
    if (!urlpattern_parse_pattern_from_string(&matchVal, &mSpec, options,                                                                                                    
                                              &pattern)) {
      return false;                                 // OK — nothing allocated                                                                                                
    }                                                                                                                                                                        
    if (urlpattern_get_has_regexp_groups(pattern)) {
      return false;                                 // LEAK — allocated, not freed                                                                                           
    }                                                                                                                                                                        
    RefPtr<DictionaryCache> dicts(DictionaryCache::GetInstance());
    if (!dicts) { return false; }                   // LEAK                                                                                                                  
    …                                                                                                                                                                        
    dicts->AddEntry(mURI, key, matchVal, matchDestItems, matchIdVal,                                                                                                         
                    Some(hash), aModified, expTime,                                                                                                                          
                    getter_AddRefs(mDictSaving));   // takes matchVal (nsACString), NOT pattern                                                                              
    …                                                                                                                                                                        
    return true;                                    // LEAK — success path                                                                                                   
  }                                                                                                                                                                          
  return true;                                      // OK    
}                                                                                                                                                                            

• UrlPatternGlue is #[repr(transparent)] pub struct UrlPatternGlue(pub *mut c_void); at netwerk/base/urlpattern_glue/src/base.rs:42 — raw pointer, no Drop. Ownership
  stays with the C++ caller.
• urlpattern_parse_pattern_from_string does Box::into_raw(Box::new(pattern)) (helpers.rs:63). The only function that reverses it is urlpattern_pattern_free
  (lib.rs:51). ParseDictionary never calls it.
• DictionaryCache::AddEntry (Dictionary.cpp:1136) takes the pattern as const nsACString& and re-parses it in DictionaryCacheEntry::Match (Dictionary.cpp:178), so the
  UrlPatternGlue is orphaned.
• Correct callers for reference: dom/urlpattern/URLPattern.cpp:162 and netwerk/cache2/Dictionary.cpp:111.

Amplification: GC root pinning

The leaked box transitively keeps up to 8 SpiderMonkeyRegexp instances alive. Each holds a *mut RegExpObjWrapper pointing to a C++ RegExpObjImpl
(URLPatternGlue.cpp:273–280) that owns a JS::PersistentRooted<JSObject*> over the SpiderMonkey RegExp object produced by parse_regexp_ffi. So the leak permanently grows
the JS runtime's persistent root set — visible in …/class(RegExp)/objects/gc-heap.

Attack model

• Precondition: Nightly default pref + any HTTPS visit.
• Action: origin serves any response with Use-As-Dictionary: match="…". Any pattern that parses successfully leaks.
• Cost: ~448 B rooted GC-heap + ~1–2 KiB native heap per trigger. No user interaction, no CORS gate (response header).
• Impact: persistent memory exhaustion / DoS, plus linear GC-scan-time degradation from root-set growth. No code execution or info disclosure.

Suggested fix

Minimal scope-based guard, attached as patch.diff:

+    auto freePattern = MakeScopeExit([&] { urlpattern_pattern_free(pattern); });

Inserted right after the successful parse so it covers the 3 leaking exits. A reviewer may prefer a UniquePtr wrapper or explicit urlpattern_pattern_free at each return.
Attached test_dictionary_ParseDictionary_leak.js is an xpcshell regression-test sketch (not run locally).

Expected results:

nsHttpChannel::ParseDictionary should call urlpattern_pattern_free(pattern) on every return path that follows a successful urlpattern_parse_pattern_from_string — the
regexp-groups rejection, the DictionaryCache-not-available fallback, and the success path. The RegExp gc-heap counter in the XPConnect junk compartment should return to
baseline after Minimize memory usage, regardless of how many Use-As-Dictionary responses were processed.

Comment 1

[ukyo]

Attachment: test_dictionary_ParseDictionary_leak.js

Comment 2

[ukyo]

Attachment: memory-report.json.gz

Comment 3

[ukyo]

Attachment: memory-report.json2.gz

Comment 4

[ukyo]

Attachment: server.py

Comment 5

[Andrew McCreight [:mccr8]]

Thank you for the report. Leaks are bad, but there are plenty of ways to leak memory with a web page.

Comment 6

[Kershaw Chang [:kershaw]]

Hi Randell,

Could you take a look?

Thanks.

Comment 8

[Andrew McCreight [:mccr8]]

Can somebody please investigate this? It is happening extremely frequently on WPTs.

Comment 9

[Kershaw Chang [:kershaw]]

Based on this try push, I can confirm that the suggested fix works.
I'll submit a patch.

Comment 10

[Kershaw Chang [:kershaw]]

Attachment: Bug 2034558 - Fix UrlPatternGlue leak in nsHttpChannel::ParseDictionary, r=jesup

Comment 11

[Andrew McCreight [:mccr8]]

(In reply to Kershaw Chang [:kershaw] from comment #9)

Based on this try push, I can confirm that the suggested fix works.
I'll submit a patch.

Thank you! I confirmed that ASan job does run one of the directories that is leaking, retriggered it 7 more times, and confirmed that "gkrust" doesn't appear anywhere in the log, so we're not just covering it up as far as I can tell.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6797110f58b6
https://hg.mozilla.org/integration/autoland/rev/066b1f6caf74
Fix UrlPatternGlue leak in nsHttpChannel::ParseDictionary, r=jesup,necko-reviewers

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/066b1f6caf74