AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert "Calls to {console} require an access check for the provided arguments"

Browse Source 
This reverts commit a5fd60e15a.

Reason for revert: As per crbug/1213374 this is not applied consistently. E.g. wrapping object into an array will bypass access checks. With the crrev/c/3041424 however, only accessible properties are shown in console, so logging a restricted object is no longer unsafe.

Original change's description:
> Calls to {console} require an access check for the provided arguments
>
> This CL adds an access check for the arguments to all calls to
> {console} like {console.log}. This is needed since the DevTools
> protocol notificiation event does not contain the context in which
> the {console.log} call occurred. Only the context of the argument.
> When DevTools then reads properties for the preview of the argument,
> it uses arguments context, instead of the calling context, potentially
> leaking objects/exceptions into the calling context.
>
> Bug: chromium:987502, chromium:986393
> Change-Id: I6f7682f7bee94a28ac61994bad259bd003511c39
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1741664
> Commit-Queue: Simon Zünd <szuend@chromium.org>
> Reviewed-by: Yang Guo <yangguo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#63122}

Bug: chromium:987502, chromium:986393, chromium:1213374
Change-Id: I92a8bb7663ff97de8831ddeb2c8560fb9fa1c12e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3046189
Reviewed-by: Simon Zünd <szuend@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Danil Somsikov <dsv@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75881}
This commit is contained in:
Danil Somsikov 2021-07-23 08:44:17 +00:00 committed by V8 LUCI CQ
parent 40b20c9401
commit 52f1d69eca
2 changed files with 0 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/builtins/builtins-console.cc
View File

@ -46,22 +46,6 @@ void ConsoleCall(
CHECK(!isolate->has_scheduled_exception());
if (!isolate->console_delegate()) return;
HandleScope scope(isolate);
// Access check. The current context has to match the context of all
// arguments, otherwise the inspector might leak objects across contexts.
Handle<Context> context = handle(isolate->context(), isolate);
for (int i = 0; i < args.length(); ++i) {
Handle<Object> argument = args.at<Object>(i);
if (!argument->IsJSObject()) continue;
Handle<JSObject> argument_obj = Handle<JSObject>::cast(argument);
if (argument->IsAccessCheckNeeded(isolate) &&
!isolate->MayAccess(context, argument_obj)) {
isolate->ReportFailedAccessCheck(argument_obj);
return;
}
}
debug::ConsoleCallArguments wrapper(args);
Handle<Object> context_id_obj = JSObject::GetDataProperty(
args.target(), isolate->factory()->console_context_id_symbol());

42
test/unittests/api/access-check-unittest.cc
View File

@ -179,46 +179,4 @@ TEST_F(AccessRegressionTest,
ASSERT_EQ(getter_c2->native_context(), *Utils::OpenHandle(*context2));
}
namespace {
bool failed_access_check_callback_called;
class AccessCheckTestConsoleDelegate : public debug::ConsoleDelegate {
public:
void Log(const debug::ConsoleCallArguments& args,
const debug::ConsoleContext& context) {
FAIL();
}
};
} // namespace
// Ensure that {console.log} does an access check for its arguments.
TEST_F(AccessCheckTest, ConsoleLog) {
isolate()->SetFailedAccessCheckCallbackFunction(
[](v8::Local<v8::Object> host, v8::AccessType type,
v8::Local<v8::Value> data) {
failed_access_check_callback_called = true;
});
AccessCheckTestConsoleDelegate console{};
debug::SetConsoleDelegate(isolate(), &console);
Local<ObjectTemplate> object_template = ObjectTemplate::New(isolate());
object_template->SetAccessCheckCallback(AccessCheck);
Local<Context> context1 = Context::New(isolate(), nullptr);
Local<Context> context2 = Context::New(isolate(), nullptr);
Local<Object> object1 =
object_template->NewInstance(context1).ToLocalChecked();
EXPECT_TRUE(context2->Global()
->Set(context2, v8_str("object_from_context1"), object1)
.IsJust());
Context::Scope context_scope(context2);
failed_access_check_callback_called = false;
CompileRun(isolate(), "console.log(object_from_context1);").ToLocalChecked();
ASSERT_TRUE(failed_access_check_callback_called);
}
} // namespace v8