Bug 312669 – DFG `operationStringProtoFuncReplaceAllGeneric` Skips Global Flag Check for RegExp

• Component: JavaScriptCore
• Status: RESOLVED FIXED

Created attachment 479191 [details]
poc.js

## Summary

`String.prototype.replaceAll` must throw a TypeError when its search argument is a non-global RegExp. The interpreter and specialized DFG operations enforce this, but `operationStringProtoFuncReplaceAllGeneric` — reached when the replacer is a function — skips the check entirely, producing a classic unopt/opt discrepancy.

**While I think this is a simple correctness bug, I thought it would be safe to report this as a security bug. Feel free to demote this to a bug.**

## Bug

### Summary

When the DFG compiles a `StringReplaceAll` node whose `child2` speculates as `RegExpObject` but `child3` is a function (not String), the fixup phase leaves all edges as `UntypedUse`. The resulting `compileStringReplace` call emits `operationStringProtoFuncReplaceAllGeneric`, which delegates directly to `replace<StringReplaceMode::Global>`. That function dispatches to `replaceUsingRegExpSearch` upon seeing a `RegExpObject` without checking the global flag, silently performing a single-match replacement instead of throwing.

### Detail

`operationStringProtoFuncReplaceAllGeneric` unconditionally delegates:

```cpp
// Source/JavaScriptCore/dfg/DFGOperations.cpp:3558-3566
JSC_DEFINE_JIT_OPERATION(operationStringProtoFuncReplaceAllGeneric, JSCell*, (JSGlobalObject* globalObject, EncodedJSValue thisValue, EncodedJSValue searchValue, EncodedJSValue replaceValue))
{
    VM& vm = globalObject->vm();
    CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
    JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
    auto scope = DECLARE_THROW_SCOPE(vm);

    OPERATION_RETURN(scope, replace<StringReplaceMode::Global>(vm, globalObject, JSValue::decode(thisValue), JSValue::decode(searchValue), JSValue::decode(replaceValue)));
}
```

Inside `replace<StringReplaceMode::Global>`, the RegExp branch jumps straight to `replaceUsingRegExpSearch` with no global flag check:

```cpp
// Source/JavaScriptCore/runtime/StringPrototypeInlines.h:1589-1590
if (searchValue.inherits<RegExpObject>())
    RELEASE_AND_RETURN(scope, replaceUsingRegExpSearch(vm, globalObject, string, searchValue, replaceValue));
```

(`StringReplaceMode::Global` only affects the plain-string search path lower in the function, not this branch.)

The specialized operations used for the `RegExpObjectUse`+`StringUse` path both carry the required check:

```cpp
// Source/JavaScriptCore/dfg/DFGOperations.cpp:3599-3602 (operationStringProtoFuncReplaceAllRegExpEmptyStr)
if (!regExp->global()) [[unlikely]] {
    throwTypeError(globalObject, scope, "String.prototype.replaceAll argument must not be a non-global regular expression"_s);
    OPERATION_RETURN(scope, nullptr);
}

// Source/JavaScriptCore/dfg/DFGOperations.cpp:3633-3636 (operationStringProtoFuncReplaceAllRegExpString)
if (!searchValue->regExp()->global()) [[unlikely]] {
    throwTypeError(globalObject, scope, "String.prototype.replaceAll argument must not be a non-global regular expression"_s);
    OPERATION_RETURN(scope, nullptr);
}
```

The interpreter path likewise checks (Source/JavaScriptCore/runtime/StringPrototype.cpp:408-409):

```cpp
if (!regExpObject->regExp()->global()) [[unlikely]]
    return throwVMTypeError(globalObject, scope, "String.prototype.replaceAll argument must not be a non-global regular expression"_s);
```

The gap is introduced by the fixup phase. The optimized (RegExpObject+String) edge fixup requires all three children to speculate correctly:

```cpp
// Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:1796-1804
if (node->child1()->shouldSpeculateString()
    && node->child2()->shouldSpeculateRegExpObject()
    && node->child3()->shouldSpeculateString()) {
    fixEdge<StringUse>(node->child1());
    fixEdge<RegExpObjectUse>(node->child2());
    fixEdge<StringUse>(node->child3());
    break;
}
```

When `child3` is a function, none of the specialized paths apply, edges remain `UntypedUse`, and `compileStringReplace` emits the generic call:

```cpp
// Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:13861-13873
case UntypedUse: {
    // ...
    callOperation(node->op() == StringReplaceAll ? operationStringProtoFuncReplaceAllGeneric : operationStringProtoFuncReplaceGeneric, ...);
    // ...
}
```

### Trigger Conditions

1. A function calls `replaceAll(regexp, fn)` where the second argument is a function (not a string).
2. It is warmed up enough times with a **global** RegExp to trigger DFG compilation (`child2` speculates as `RegExpObject`, `child3` as non-String → `UntypedUse` path).
3. The RegExp primordial properties watchpoint is intact.
4. The DFG-compiled function is then called with a **non-global** RegExp.

## Version

### Reproduced Version

- `main` branch latest commit (2026/04/18): `a4390137a4038c07661f58e45ee205b5a623f9dd`

## Reproduction Case

### Release Build (Debug build produces identical output)

```bash
jsc poc.js
```

Result:

```
interpreter: String.prototype.replaceAll argument must not be a non-global regular expression
dfg:         hellO world
```

### PoC Code

```js
// replaceAll must throw TypeError for non-global RegExp.
// DFG-compiled path skips this check — unopt/opt discrepancy.

function doReplaceAll(str, pattern, replacer) {
    return str.replaceAll(pattern, replacer);
}

// Interpreter: throws TypeError before any warmup
try { doReplaceAll("hello world", /o/, function(m) { return m.toUpperCase(); }); }
catch (e) { print("interpreter: " + e.message); }

// Warm up: RegExpObject + function replacer → UntypedUse → operationStringProtoFuncReplaceAllGeneric
for (var i = 0; i < 200000; i++)
    doReplaceAll("hello world", /o/g, function(m) { return m.toUpperCase(); });

// DFG: silently performs single-match replacement (bug)
try { print("dfg:         " + doReplaceAll("hello world", /o/, function(m) { return m.toUpperCase(); })); }
catch (e) { print("dfg:         " + e.message); }
```

## Suggested Patch

```diff
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -3560,7 +3560,15 @@ JSC_DEFINE_JIT_OPERATION(operationStringProtoFuncReplaceAllGeneric, JSCell*, (JS
     VM& vm = globalObject->vm();
     CallFrame* callFrame = DECLARE_CALL_FRAME(vm);
     JITOperationPrologueCallFrameTracer tracer(vm, callFrame);
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    OPERATION_RETURN(scope, replace<StringReplaceMode::Global>(vm, globalObject, JSValue::decode(thisValue), JSValue::decode(searchValue), JSValue::decode(replaceValue)));
+    JSValue decodedSearchValue = JSValue::decode(searchValue);
+    if (decodedSearchValue.inherits<RegExpObject>()) [[unlikely]] {
+        if (!jsCast<RegExpObject*>(decodedSearchValue)->regExp()->global()) {
+            throwTypeError(globalObject, scope, "String.prototype.replaceAll argument must not be a non-global regular expression"_s);
+            OPERATION_RETURN(scope, nullptr);
+        }
+    }
+
+    OPERATION_RETURN(scope, replace<StringReplaceMode::Global>(vm, globalObject, JSValue::decode(thisValue), decodedSearchValue, JSValue::decode(replaceValue)));
 }
```

Mirrors the checks already in `operationStringProtoFuncReplaceAllRegExpEmptyStr` (line 3599) and `operationStringProtoFuncReplaceAllRegExpString` (line 3633).

### Credit Information

Reporter credit: Junyoung Park (@candymate) of KAIST Hacking Lab

[Radar WebKit Bug Importer]

<rdar://problem/175067313>

[Shu-yu Guo]

This is a correctness bug, not a security bug.

[Kai Tamkun]

Pull request: https://github.com/WebKit/WebKit/pull/64290

[EWS]

Committed 313027@main (9cfacfaca909): <https://commits.webkit.org/313027@main>

Reviewed commits have been landed. Closing PR #64290 and removing active labels.