1564362 - A manipulated regular expression object passed to "String.prototype.match" can make JavaScript engine stuck.

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[mir597]

VULNERABILITY DETAILS
This simple code can make JavaScript interpreter stuck.
Thus, it happens all the implementations that use the same JavaScript engine.

I believe that it happens due to a developer forgot to upgrade String.prototype.match from ECMAScript 5.1.

In ECMAScript 5.1, a regular expression object must have its own properties such as "global", "ignoreCase", and so on (defined at https://www.ecma-international.org/ecma-262/5.1/#sec-15.10.7)
But, later version of ECMAScript, it moved to its prototype object (defined at https://www.ecma-international.org/ecma-262/6.0/#sec-properties-of-regexp-instances)

In the meantime, the implementation of String.prototype.match considers old semantics (defined at https://www.ecma-international.org/ecma-262/5.1/#sec-15.5.4.10)
In steps 7 and 8, it considers "global" property must be either "true" or "false".
However, since the "global" property is moved to its prototype object, a user can define "global" property and its value can be another value (in my example, I use "idk" string value).

Since the value is neither "true" nor "false", the JavaScript engine just stop to interpret the code.
If you use another regular expression object, such as /reg/g, it will be simply passed.
I believe that it happens due to optimization. It knows that the object's "global" property is true by the syntax.

VERSION
I've only tested in old version "66.0.3", but I'm pretty sure that it will exist in the latest version.
This bug exists in other JavaScript engines (V8, JavaScriptCore - from safari).

REPRODUCTION CASE

var o = /reg/;
Object.defineProperty(o, "global", {value: "idk", writable: true, configurable: true, enumerable: true});
"regular expression?".match(o);

Thank you.

Comment 1

[mir597]

Attachment: bug.js

This code will raise the buggy situation.
Please check the code.

Comment 2

[mir597]

I forgot to mention how to fix the bug.

You can simply fix the bug by applying "toBoolean" to the value of 'global' property.
The related specification is here:

• https://www.ecma-international.org/ecma-262/10.0/index.html#sec-regexp.prototype-@@match
• the step 4.

Comment 3

[Johann Hofmann [:johannh]]

Very interesting, forwarding to JS folks. Jason, can you get this in the right hands, please? Thanks!

Comment 4

[André Bargull [:anba]]

This behaviour is expected by the specification:

• 21.2.5.7 RegExp.prototype [ @@match ] uses the user-controllable "global" property.
• Whereas 21.2.5.2.2 Runtime Semantics: RegExpBuiltinExec uses the internal slot [[OriginalFlags]] to compute the global state.

That means for the test case, that 21.2.5.7 RegExp.prototype [ @@match ] will use global = true, because ToBoolean applied on the string "idk" is true, but 21.2.5.2.2 Runtime Semantics: RegExpBuiltinExec keeps using global = false, because [[OriginalFlags]] = "", so it doesn't contain the string "g", cf. 21.2.5.2.2 step 6.

Furthermore RegExpBuiltinExec will always search for a match starting at index 0, cf. 21.2.5.2.2 step 8, so in the test case there'll always be a match, which leads to never breaking from the loop in 21.2.5.7, step 6.f, see the stop condition in step 6.f.ii.

Comment 5

[Jason Orendorff [:jorendorff]]

OK, so the behavior is per spec. The slow script UI works. It's just an infinite loop.

Thank you for the report, mir597, but this isn't any more security-sensitive than while (true) {}.

Setting RESOLVED INVALID, which just means it's not a bug. Or not our bug. I don't think the ECMAScript committee would be interested in changing the standard here though. It seems like the Get(rx, "global") in 21.2.5.7 is probably meant to support subclassing RegExp, but it doesn't actually work. That isn't ideal, but maybe not worth changing an established standard to rectify. After all nobody actually wants to subclass RegExp.

Comment 6

[mir597]

Thank you for confirming the situation.
Yes, it is something I should talk with the ECMAScript committees.

But, will you plan to leave the bug as it is?
As long as you leave it, any malicious user can exploit this and break "the availability property" of a sensitive program and makes some security problem.

Let's assume that there is a sensitive operation that has two consequent operations (for example, "open the door" and "close the door").
Once a malicious user can manipulate the regular expression object and the sensitive code uses the object to "match" function in the middle of the operations, the malicious user can make the door open left.
(I know JavaScript engine is not used for such sensitive operations in practice so far, but as long as it is started to use for developing standalone applications, I believe that it is worth to consider this case.)

I don't encourage you to consider this report as a serious security bug, but I believe that it is worth to fix the infinite loop in the security point of view.