1939488 - "InternalError: too much recursion" when loading a large manifest.json file

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P2)

Description

[Rob Wu [:robwu]]

STR:

1. Create a manifest.json and add a key with many characters. E.g. 10M of "foo": "X (repeated 10M times)"
2. Load it via about:debugging

Expected:

• The extension should load.

Actual:

• The extension fails to load, with about:debugging's UI displaying:

There was an error during the temporary add-on installation.
Error details

too much recursion`

• And the error console:

[ACTION FAILED] TEMPORARY_EXTENSION_INSTALL_FAILURE: too much recursion
readJSON/</<@resource://gre/modules/Extension.sys.mjs:1174:25
onStopRequest@resource://gre/modules/NetUtil.sys.mjs:124:18

Comment 1

[Rob Wu [:robwu]]

This issue is caused by the COMMENT_REGEXP at https://searchfox.org/mozilla-central/rev/222add4dd36e034fa4d0a8b3e4e1ded979599892/toolkit/components/extensions/Extension.sys.mjs#398-411

This regular expression contains a repeating non-capturing group, but with large inputs, the internal implementation seems to recurse to a depth proportional to the string to test.

I tried fixing the COMMENT_REGEXP regexp to reduce backtracking (beyond what was attempted in bug 1299256), but found that even the simplest of regular expressions can exhaust the stack when a large input is fed. E.g. the following regexp can be used to try and match a string literal: /^"(?:[^"\\\n]|\\.)*"/, but triggers RangeError when passed '"' + 'x'.repeat(1e7) + '"'

Here is a self-contained test case demonstrating that a repeating non-capturing group can lead to deep recursion depths, and thus trigger an error (in Chromium and Firefox):

/^a(?:[^x]|nevermatch)*/.exec('a' + "b".repeat(1e7) +'c')

Comment 2

[Rob Wu [:robwu]]

Attachment: Bug 1939488 - Replace inefficient regex of JSON comment stripper with linear algorithm

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:zombie, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:robwu, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38de86874c87
Replace inefficient regex of JSON comment stripper with linear algorithm r=zombie

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/38de86874c87