Commit 720d5fc1 authored by David Tseng's avatar David Tseng Committed by Commit Bot

Fixes crash in chrome.automation

After
https://chromium-review.googlesource.com/c/chromium/src/+/2424854

we slightly changed some of the enum key literals e.g. activedecsendantchanged
to activeDescendantChanged.

This undercovered an issue where methods like

chrome.automation.AutomationNode.prototype.addEventListener

appear to receive no validation. Callers can do
|node.addEventListener(undefined)| and reach the native bindings.

This change is a quick fix to make it so we don't crash.

R=dmazzoni@chromium.org

AX-Relnotes: n/a
Fixed: 1132662
Change-Id: Idf4effeb29a6974df6d2d0a933f157c3aed705ad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2433997
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811495}
parent 9e754fbe
...@@ -54,15 +54,19 @@ namespace extensions { ...@@ -54,15 +54,19 @@ namespace extensions {
namespace { namespace {
void ThrowInvalidArgumentsException( void ThrowInvalidArgumentsException(
AutomationInternalCustomBindings* automation_bindings) { AutomationInternalCustomBindings* automation_bindings,
bool is_fatal = true) {
v8::Isolate* isolate = automation_bindings->GetIsolate(); v8::Isolate* isolate = automation_bindings->GetIsolate();
automation_bindings->GetIsolate()->ThrowException( automation_bindings->GetIsolate()->ThrowException(
v8::String::NewFromUtf8Literal( v8::String::NewFromUtf8Literal(
isolate, isolate,
"Invalid arguments to AutomationInternalCustomBindings function")); "Invalid arguments to AutomationInternalCustomBindings function"));
LOG(FATAL) << "Invalid arguments to AutomationInternalCustomBindings function" if (is_fatal) {
<< automation_bindings->context()->GetStackTraceAsString(); LOG(FATAL)
<< "Invalid arguments to AutomationInternalCustomBindings function"
<< automation_bindings->context()->GetStackTraceAsString();
}
} }
v8::Local<v8::String> CreateV8String(v8::Isolate* isolate, v8::Local<v8::String> CreateV8String(v8::Isolate* isolate,
...@@ -534,7 +538,12 @@ class NodeIDPlusEventWrapper ...@@ -534,7 +538,12 @@ class NodeIDPlusEventWrapper
v8::Isolate* isolate = automation_bindings_->GetIsolate(); v8::Isolate* isolate = automation_bindings_->GetIsolate();
if (args.Length() < 3 || !args[0]->IsString() || !args[1]->IsInt32() || if (args.Length() < 3 || !args[0]->IsString() || !args[1]->IsInt32() ||
!args[2]->IsString()) { !args[2]->IsString()) {
ThrowInvalidArgumentsException(automation_bindings_); // The extension system does not perform argument validation in js, so an
// extension author can do something like node.addEventListener(undefined)
// and reach here. Do not crash the process.
ThrowInvalidArgumentsException(automation_bindings_,
false /* is_fatal */);
return;
} }
ui::AXTreeID tree_id = ui::AXTreeID tree_id =
...@@ -542,6 +551,11 @@ class NodeIDPlusEventWrapper ...@@ -542,6 +551,11 @@ class NodeIDPlusEventWrapper
int node_id = args[1].As<v8::Int32>()->Value(); int node_id = args[1].As<v8::Int32>()->Value();
auto event_type = api::automation::ParseEventType( auto event_type = api::automation::ParseEventType(
*v8::String::Utf8Value(isolate, args[2])); *v8::String::Utf8Value(isolate, args[2]));
if (event_type == api::automation::EVENT_TYPE_NONE) {
ThrowInvalidArgumentsException(automation_bindings_,
false /* is_fatal */);
return;
}
AutomationAXTreeWrapper* tree_wrapper = AutomationAXTreeWrapper* tree_wrapper =
automation_bindings_->GetAutomationAXTreeWrapperFromTreeID(tree_id); automation_bindings_->GetAutomationAXTreeWrapperFromTreeID(tree_id);
......
...@@ -1049,12 +1049,17 @@ AutomationNodeImpl.prototype = { ...@@ -1049,12 +1049,17 @@ AutomationNodeImpl.prototype = {
this.removeEventListener(eventType, callback); this.removeEventListener(eventType, callback);
if (!this.listeners[eventType]) if (!this.listeners[eventType])
this.listeners[eventType] = []; this.listeners[eventType] = [];
// Calling EventListenerAdded will also validate the args
// and throw an exception it's not a valid event type, so no invalid event
// type/listener gets enqueued.
EventListenerAdded(this.treeID, this.id, eventType);
$Array.push(this.listeners[eventType], { $Array.push(this.listeners[eventType], {
__proto__: null, __proto__: null,
callback: callback, callback: callback,
capture: !!capture, capture: !!capture,
}); });
EventListenerAdded(this.treeID, this.id, eventType);
}, },
// TODO(dtseng/aboxhall): Check this impl against spec. // TODO(dtseng/aboxhall): Check this impl against spec.
......
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