Created attachment 165669 [details] poc for iOS 5.1.1 ZDI-CAN-1657: (Mobile Pwn2Own) WebKit Shiftcount Remote Code Execution Vulnerability -- CVSS ----------------------------------------- 7.5, AV:N/AC:L/Au:N/C:P/I:P/A:P -- ABSTRACT ------------------------------------- TippingPoint has identified a vulnerability affecting the following products: WebKit -- CREDIT --------------------------------------- This vulnerability was discovered by: Joost Pol, Certified Secure Daan Keuper, Certified Secure
Abstract: Vulnerability in shiftCount/splice ArrayPrototype.cpp::arrayProtoFuncSplice can be tricked into passing a "count" >= array.length to JSArray.cpp::shiftCount. This in turn can be used to trick JSArray.cpp::shiftCount into shifting an array beyond its length and/or shifting array with sparse values in it. This leads to an exploitable scenario in both IOS 5.1.1 and IOS 6. It is debatle wether the vulnerability is in arrayProtoFuncSplice (which contains a simple toctou) or in shiftCount (which does not validate its passed parameters correctly). We would suggest patching both shiftCount (validating count) and splice (fixup the toctou). JavaScriptCore/runtime/ArrayProtoType.cpp::arrayProtoFuncSplice The "deleteCount" is checked against the array length, but the array length can be changed from the toInteger callback. See the code below from the arrayProtoFuncSplice function (IOS 5.1.1 fragment): Simple toctou in the splice routine: unsigned length = thisObj->get(exec, exec->propertyNames().length).toUInt32(exec); ... double deleteDouble = exec->argument(1).toInteger(exec); // * .. if (deleteDouble > length - begin) deleteCount = length - begin; .. ((JSArray *)thisObj)->shiftCount(exec, deleteCount - additionalArgs); *) toInteger -> interrupt and shrink thisObj (making deleteCount >= array.length) JavaScriptCore/runtime/JSArray.cpp::shiftCount Both IOS5.1.1 and IOS 6: When a "count" > length is passed both the numValuesInVector and m_length members will wrap-around leading to an exploitable scenario. Specific to IOS 5.1.1: When passing a count == length *and* there are sparse-values in the map, the length can be shifted *without* removing/invalidating the sparse-value entries. exploitPath: IOS 5.1.1 Straightforward exploitation on IOS 5.1.1. The exploit path we chose for IOS 5.1.1 uses the above situation to setup an array with valid values in the vector but where the array's m_length < m_NumValuesInVector. When GC kicks in, the values in the vector will not be marked (see visitChildren). Since we can still access the values (see getIndex) we have a classic use-after-free. This use-after-free can be abused in multiple ways and will most certainly lead to code execution. In the exploit demonstrated at the pwn2own we chose to use the dangling references to shift the "resObj" local-variable in the splice routine. This will trigger another memory-overwrite. The use-after-free is illustarted in the POC. exploitPath: IOS 6 More complicated exploitation on IOS 6: Garbage colletion and allocations work differently. Important differnce is also that shiftCount does not allowing shifting when there are sparse values in the map: if (oldLength != storage->m_numValuesInVector ...) return; First we trigger the vulnerability to setup an (empty) array where the m_numValuesInVector wrapped to -1 (0xFFFFFFFF). Next we insert some sparse-value entries at the end of the map (0xFFFFFFFE and down). Important: Nos we set the array length to 0xFFFFFFF, this does not delete any values from the map (*increment* of length) but does make sure that m_length == mNumValuesInVector (both -1). Now we can trigger the splice-routine again, but this time *with* some values in the sparsemap. This will decrease the length again (*below* our sparse-value-indexes) but will leave our values untouched. For clarity we set the length to zero (changes nothing). Now we again have an array with sparse-values in it but with a zero length. Inserting non-sparse values into this array will trigger a sparse-map -> vector move in putBeyondIndex The vector will be (re)allocated based on the (faulty) length and the values from the sparse map will be moved into the vector. Since we are using large indexes (X - 0xFFFFFFE) this will actually start *underflowing* the m_vector array: vector[it->first].set(globalData, this, it->second.getNonSparseMode()); This underflow can be abused easiliy and will most certainly lead to code execution. The path we confirmed for IOS6 uses the underflow to overwrite (with an atrbitrary address) the "allocBase" of another array. Whenever a reallocation of this array is triggered, values will be copied into the array from the address we specified. This in turn gives us a nice info-leak but also leads directly to code execution since we can just copy abtitrary values/cell into the vector and access them. The underflow and info-leak vector are illustrated in the POCs.
<rdar://problem/12370864>
Created attachment 165670 [details] poc of iOS 6.0 crash
Created attachment 165671 [details] poc of iOS 6.0 leak
looks like a JSC bug.
Created attachment 165696 [details] the patch
Comment on attachment 165696 [details] the patch You might want to put an ASSERT inside the array shift/unshift methods to check count is sane for current length. r+
(In reply to comment #7) > (From update of attachment 165696 [details]) > You might want to put an ASSERT inside the array shift/unshift methods to check count is sane for current length. r+ Yup, I've done that. It doesn't break things.
Landed in http://trac.webkit.org/changeset/129577