Commit 1da4b373 authored by Stephen Roettger's avatar Stephen Roettger Committed by V8 LUCI CQ

Replace more args.set_at usages with ChangeValueScope

args.set_at lead to a vulnerability in the past where the caller
(ignition) didn't expect the callee to overwrite the arguments.

The current usage doesn't look like an issue, but let's preemptively
remove these usages so that they don't lead to issues in the future.

Change-Id: I64e1f84ad1833b2b2f96cd7503bdde00f344404c
Bug: chromium:1268738
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3644965Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Commit-Queue: Stephen Röttger <sroettger@google.com>
Cr-Commit-Position: refs/heads/main@{#80641}
parent 44e1d129
...@@ -54,6 +54,7 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> HandleApiCallHelper( ...@@ -54,6 +54,7 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> HandleApiCallHelper(
Handle<Object> receiver, BuiltinArguments args) { Handle<Object> receiver, BuiltinArguments args) {
Handle<JSReceiver> js_receiver; Handle<JSReceiver> js_receiver;
JSReceiver raw_holder; JSReceiver raw_holder;
base::Optional<BuiltinArguments::ChangeValueScope> set_receiver_value_scope;
if (is_construct) { if (is_construct) {
DCHECK(args.receiver()->IsTheHole(isolate)); DCHECK(args.receiver()->IsTheHole(isolate));
if (fun_data->GetInstanceTemplate().IsUndefined(isolate)) { if (fun_data->GetInstanceTemplate().IsUndefined(isolate)) {
...@@ -70,7 +71,8 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> HandleApiCallHelper( ...@@ -70,7 +71,8 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> HandleApiCallHelper(
ApiNatives::InstantiateObject(isolate, instance_template, ApiNatives::InstantiateObject(isolate, instance_template,
Handle<JSReceiver>::cast(new_target)), Handle<JSReceiver>::cast(new_target)),
Object); Object);
args.set_at(0, *js_receiver); set_receiver_value_scope.emplace(
isolate, &args, BuiltinArguments::kReceiverOffset, *js_receiver);
DCHECK_EQ(*js_receiver, *args.receiver()); DCHECK_EQ(*js_receiver, *args.receiver());
raw_holder = *js_receiver; raw_holder = *js_receiver;
...@@ -211,8 +213,8 @@ MaybeHandle<Object> Builtins::InvokeApiFunction(Isolate* isolate, ...@@ -211,8 +213,8 @@ MaybeHandle<Object> Builtins::InvokeApiFunction(Isolate* isolate,
argv[BuiltinArguments::kArgcOffset] = Smi::FromInt(frame_argc).ptr(); argv[BuiltinArguments::kArgcOffset] = Smi::FromInt(frame_argc).ptr();
argv[BuiltinArguments::kPaddingOffset] = argv[BuiltinArguments::kPaddingOffset] =
ReadOnlyRoots(isolate).the_hole_value().ptr(); ReadOnlyRoots(isolate).the_hole_value().ptr();
int cursor = BuiltinArguments::kNumExtraArgs; argv[BuiltinArguments::kReceiverOffset] = receiver->ptr();
argv[cursor++] = receiver->ptr(); int cursor = BuiltinArguments::kNumExtraArgsWithReceiver;
for (int i = 0; i < argc; ++i) { for (int i = 0; i < argc; ++i) {
argv[cursor++] = args[i]->ptr(); argv[cursor++] = args[i]->ptr();
} }
......
...@@ -1515,7 +1515,8 @@ BUILTIN(ArrayConcat) { ...@@ -1515,7 +1515,8 @@ BUILTIN(ArrayConcat) {
ASSIGN_RETURN_FAILURE_ON_EXCEPTION( ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, receiver, isolate, receiver,
Object::ToObject(isolate, args.receiver(), "Array.prototype.concat")); Object::ToObject(isolate, args.receiver(), "Array.prototype.concat"));
args.set_at(0, *receiver); BuiltinArguments::ChangeValueScope set_receiver_value_scope(
isolate, &args, BuiltinArguments::kReceiverOffset, *receiver);
Handle<JSArray> result_array; Handle<JSArray> result_array;
......
...@@ -20,7 +20,10 @@ Handle<Object> BuiltinArguments::atOrUndefined(Isolate* isolate, ...@@ -20,7 +20,10 @@ Handle<Object> BuiltinArguments::atOrUndefined(Isolate* isolate,
return at<Object>(index); return at<Object>(index);
} }
Handle<Object> BuiltinArguments::receiver() const { return at<Object>(0); } Handle<Object> BuiltinArguments::receiver() const {
int index = kReceiverOffset;
return Handle<Object>(address_of_arg_at(index));
}
Handle<JSFunction> BuiltinArguments::target() const { Handle<JSFunction> BuiltinArguments::target() const {
int index = kTargetOffset; int index = kTargetOffset;
......
...@@ -50,6 +50,7 @@ class BuiltinArguments : public JavaScriptArguments { ...@@ -50,6 +50,7 @@ class BuiltinArguments : public JavaScriptArguments {
static constexpr int kTargetOffset = 1; static constexpr int kTargetOffset = 1;
static constexpr int kArgcOffset = 2; static constexpr int kArgcOffset = 2;
static constexpr int kPaddingOffset = 3; static constexpr int kPaddingOffset = 3;
static constexpr int kReceiverOffset = 4;
static constexpr int kNumExtraArgs = 4; static constexpr int kNumExtraArgs = 4;
static constexpr int kNumExtraArgsWithReceiver = 5; static constexpr int kNumExtraArgsWithReceiver = 5;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment