“ Detailed Report: https://clusterfuzz.com/testcase?key=4844628113031168 ”
“ Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4844628113031168 ”
“ If you have any feedback on reproducing test cases, let us know at https://forms.gle/Yh3qCYFveHj6E5jz5 so we can improve. ”
“ I think now that we're using leaptiering, this issue has become much easier to trigger for a fuzzer. Now it is enough for the fuzzer to corrupt a SharedFunctionInfo object and call a CPP builtin via the leaptiering entry. Then the
“ I think the best fix here would be to let the CPP builtin pass the expected parameter count together with the pointer to the C++ function into the AdaptorWithBuiltinExitFrame builtin. That should also be a little more performant since we can get rid of a few heap memory reads I think (of the SFI and SFI::formal_parameter_count). We also need to make sure that the
“ I think the best fix here would be to let the CPP builtin pass the expected parameter count together with the pointer to the C++ function into the AdaptorWithBuiltinExitFrame builtin. That should also be a little more performant since we can get rid of a few heap memory reads I think (of the SFI and SFI::formal_parameter_count). We also need to make sure that the parameter_count on the Code object for a CPP Builtin matches what it's adaptor uses (otherwise we should crash
“ If an attacker modifies the JSDispatchHandle between (1) and (2) (arguably a tight race, but given enough attempts, should be doable), we'll have the same problem again. A robust solution therefore would be to hardcode the expected parameter count into the builtin (which is anyway most likely better for performance). Alternatively, for other builtins (e.g. the InterpreterEntryTrampoline) we'll anyway need the caller to pass the JSDispatchEntry (or directly the parameter count) to the callee (see https://chromium-review.googlesource.com/c/v8/v8/+/5857400 which is the first step towards that). If we had that, we could also load the parameter count from there in the adaptor builtin since then we wouldn't have the double fetch: ”