⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 201495 in webkit


Ignore:
Timestamp:
May 29, 2016, 12:01:36 PM (10 years ago)
Author:
sbarati@apple.com
Message:

Stack overflow crashes with deep or cyclic proxy prototype chains
​https://bugs.webkit.org/show_bug.cgi?id=157087

Reviewed by Filip Pizlo and Mark Lam.

Because a Proxy can call back into the JS runtime in arbitrary
ways, we may have effectively cyclic prototype chains and property lookups
by using a Proxy. We may also have arbitrarily long Proxy chains
where we call into a C frame for each link in the Proxy chain.
This means that every Proxy hook must be aware that it can stack overflow.
Before, only certain hooks were aware of this fact. That was a bug,
all hooks must assume they can stack overflow.

Also, because we may have effectively cyclic prototype chains, we
compile ProxyObject.cpp with -fno-optimize-sibling-calls. This prevents
tail call optimization from happening on any of the calls from
ProxyObject.cpp. We do this because we rely on the machine stack
growing for throwing a stack overflow error. It's better for developers
to be able to see a stack overflow error than to have their program
infinite loop because the compiler performed TCO.

This patch also fixes a couple call sites of various methods
where we didn't check for an exception.

  • CMakeLists.txt:
  • JavaScriptCore.xcodeproj/project.pbxproj:
  • interpreter/Interpreter.cpp:

(JSC::sizeOfVarargs):

  • runtime/InternalFunction.cpp:

(JSC::InternalFunction::createSubclassStructure):

  • runtime/JSArray.h:

(JSC::getLength):

  • runtime/ObjectPrototype.cpp:

(JSC::objectProtoFuncToString):

  • runtime/ProxyObject.cpp:

(JSC::performProxyGet):
(JSC::ProxyObject::performInternalMethodGetOwnProperty):
(JSC::ProxyObject::performHasProperty):
(JSC::ProxyObject::getOwnPropertySlotCommon):
(JSC::ProxyObject::performPut):
(JSC::performProxyCall):
(JSC::performProxyConstruct):
(JSC::ProxyObject::performDelete):
(JSC::ProxyObject::performPreventExtensions):
(JSC::ProxyObject::performIsExtensible):
(JSC::ProxyObject::performDefineOwnProperty):
(JSC::ProxyObject::performGetOwnPropertyNames):
(JSC::ProxyObject::getOwnPropertyNames):
(JSC::ProxyObject::getPropertyNames):
(JSC::ProxyObject::getOwnNonIndexPropertyNames):
(JSC::ProxyObject::performSetPrototype):
(JSC::ProxyObject::performGetPrototype):

  • runtime/ProxyObject.h:

(JSC::ProxyObject::create):

  • tests/stress/proxy-stack-overflow-exceptions.js: Added.

(shouldThrowStackOverflow):
(const.emptyFunction):
(makeLongProxyChain):
(shouldThrowStackOverflow.longProxyChain):
(shouldThrowStackOverflow.effecivelyCyclicProxyProtoChain1):
(shouldThrowStackOverflow.effecivelyCyclicProxyProtoChain2):
(shouldThrowStackOverflow.effecivelyCyclicProxyProtoChain3):
(shouldThrowStackOverflow.longProxyChainBind):
(shouldThrowStackOverflow.longProxyChainPropertyAccess):
(shouldThrowStackOverflow.longProxyChainReflectConstruct):
(shouldThrowStackOverflow.longProxyChainReflectSet):
(shouldThrowStackOverflow.longProxyChainReflectOwnKeys):
(shouldThrowStackOverflow.longProxyChainGetPrototypeOf):
(shouldThrowStackOverflow.longProxyChainSetPrototypeOf):
(shouldThrowStackOverflow.longProxyChainGetOwnPropertyDescriptor):
(shouldThrowStackOverflow.longProxyChainDefineProperty):
(shouldThrowStackOverflow.longProxyChainIsExtensible):
(shouldThrowStackOverflow.longProxyChainPreventExtensions):
(shouldThrowStackOverflow.longProxyChainDeleteProperty):
(shouldThrowStackOverflow.longProxyChainWithScope):
(shouldThrowStackOverflow.longProxyChainWithScope2):
(shouldThrowStackOverflow.longProxyChainWithScope3):
(shouldThrowStackOverflow.longProxyChainArrayPrototypePush):
(shouldThrowStackOverflow.longProxyChainWithScope4):
(shouldThrowStackOverflow.longProxyChainCall):
(shouldThrowStackOverflow.longProxyChainConstruct):
(shouldThrowStackOverflow.longProxyChainHas):

Location:
trunk/Source/JavaScriptCore
Files:
1 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/CMakeLists.txt

    r201481 r201495  
    846846)
    847847
     848# Extra flags for compile sources can go here.
     849set_source_files_properties(runtime/ProxyObject.cpp PROPERTIES COMPILE_FLAGS -fno-optimize-sibling-calls)
     850
    848851set(JavaScriptCore_OBJECT_LUT_SOURCES
    849852    runtime/ArrayConstructor.cpp

  • trunk/Source/JavaScriptCore/ChangeLog

    r201494 r201495  
     12016-05-29  Saam barati  <sbarati@apple.com>
     2
     3        Stack overflow crashes with deep or cyclic proxy prototype chains
     4        https://bugs.webkit.org/show_bug.cgi?id=157087
     5
     6        Reviewed by Filip Pizlo and Mark Lam.
     7
     8        Because a Proxy can call back into the JS runtime in arbitrary
     9        ways, we may have effectively cyclic prototype chains and property lookups
     10        by using a Proxy. We may also have arbitrarily long Proxy chains
     11        where we call into a C frame for each link in the Proxy chain.
     12        This means that every Proxy hook must be aware that it can stack overflow.
     13        Before, only certain hooks were aware of this fact. That was a bug,
     14        all hooks must assume they can stack overflow.
     15
     16        Also, because we may have effectively cyclic prototype chains, we
     17        compile ProxyObject.cpp with -fno-optimize-sibling-calls. This prevents
     18        tail call optimization from happening on any of the calls from
     19        ProxyObject.cpp. We do this because we rely on the machine stack
     20        growing for throwing a stack overflow error. It's better for developers
     21        to be able to see a stack overflow error than to have their program
     22        infinite loop because the compiler performed TCO.
     23
     24        This patch also fixes a couple call sites of various methods
     25        where we didn't check for an exception.
     26
     27        * CMakeLists.txt:
     28        * JavaScriptCore.xcodeproj/project.pbxproj:
     29        * interpreter/Interpreter.cpp:
     30        (JSC::sizeOfVarargs):
     31        * runtime/InternalFunction.cpp:
     32        (JSC::InternalFunction::createSubclassStructure):
     33        * runtime/JSArray.h:
     34        (JSC::getLength):
     35        * runtime/ObjectPrototype.cpp:
     36        (JSC::objectProtoFuncToString):
     37        * runtime/ProxyObject.cpp:
     38        (JSC::performProxyGet):
     39        (JSC::ProxyObject::performInternalMethodGetOwnProperty):
     40        (JSC::ProxyObject::performHasProperty):
     41        (JSC::ProxyObject::getOwnPropertySlotCommon):
     42        (JSC::ProxyObject::performPut):
     43        (JSC::performProxyCall):
     44        (JSC::performProxyConstruct):
     45        (JSC::ProxyObject::performDelete):
     46        (JSC::ProxyObject::performPreventExtensions):
     47        (JSC::ProxyObject::performIsExtensible):
     48        (JSC::ProxyObject::performDefineOwnProperty):
     49        (JSC::ProxyObject::performGetOwnPropertyNames):
     50        (JSC::ProxyObject::getOwnPropertyNames):
     51        (JSC::ProxyObject::getPropertyNames):
     52        (JSC::ProxyObject::getOwnNonIndexPropertyNames):
     53        (JSC::ProxyObject::performSetPrototype):
     54        (JSC::ProxyObject::performGetPrototype):
     55        * runtime/ProxyObject.h:
     56        (JSC::ProxyObject::create):
     57        * tests/stress/proxy-stack-overflow-exceptions.js: Added.
     58        (shouldThrowStackOverflow):
     59        (const.emptyFunction):
     60        (makeLongProxyChain):
     61        (shouldThrowStackOverflow.longProxyChain):
     62        (shouldThrowStackOverflow.effecivelyCyclicProxyProtoChain1):
     63        (shouldThrowStackOverflow.effecivelyCyclicProxyProtoChain2):
     64        (shouldThrowStackOverflow.effecivelyCyclicProxyProtoChain3):
     65        (shouldThrowStackOverflow.longProxyChainBind):
     66        (shouldThrowStackOverflow.longProxyChainPropertyAccess):
     67        (shouldThrowStackOverflow.longProxyChainReflectConstruct):
     68        (shouldThrowStackOverflow.longProxyChainReflectSet):
     69        (shouldThrowStackOverflow.longProxyChainReflectOwnKeys):
     70        (shouldThrowStackOverflow.longProxyChainGetPrototypeOf):
     71        (shouldThrowStackOverflow.longProxyChainSetPrototypeOf):
     72        (shouldThrowStackOverflow.longProxyChainGetOwnPropertyDescriptor):
     73        (shouldThrowStackOverflow.longProxyChainDefineProperty):
     74        (shouldThrowStackOverflow.longProxyChainIsExtensible):
     75        (shouldThrowStackOverflow.longProxyChainPreventExtensions):
     76        (shouldThrowStackOverflow.longProxyChainDeleteProperty):
     77        (shouldThrowStackOverflow.longProxyChainWithScope):
     78        (shouldThrowStackOverflow.longProxyChainWithScope2):
     79        (shouldThrowStackOverflow.longProxyChainWithScope3):
     80        (shouldThrowStackOverflow.longProxyChainArrayPrototypePush):
     81        (shouldThrowStackOverflow.longProxyChainWithScope4):
     82        (shouldThrowStackOverflow.longProxyChainCall):
     83        (shouldThrowStackOverflow.longProxyChainConstruct):
     84        (shouldThrowStackOverflow.longProxyChainHas):
     85
    1862016-05-28  Andreas Kling  <akling@apple.com>
    287

  • trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r201481 r201495  
    12891289                79B00CBC1C6AB07E0088C65D /* ProxyConstructor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79B00CB81C6AB07E0088C65D /* ProxyConstructor.cpp */; };
    12901290                79B00CBD1C6AB07E0088C65D /* ProxyConstructor.h in Headers */ = {isa = PBXBuildFile; fileRef = 79B00CB91C6AB07E0088C65D /* ProxyConstructor.h */; settings = {ATTRIBUTES = (Private, ); }; };
    1291                 79B00CBE1C6AB07E0088C65D /* ProxyObject.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79B00CBA1C6AB07E0088C65D /* ProxyObject.cpp */; };
     1291                79B00CBE1C6AB07E0088C65D /* ProxyObject.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79B00CBA1C6AB07E0088C65D /* ProxyObject.cpp */; settings = {COMPILER_FLAGS = "-fno-optimize-sibling-calls"; }; };
    12921292                79B00CBF1C6AB07E0088C65D /* ProxyObject.h in Headers */ = {isa = PBXBuildFile; fileRef = 79B00CBB1C6AB07E0088C65D /* ProxyObject.h */; settings = {ATTRIBUTES = (Private, ); }; };
    12931293                79C4B15D1BA2158F00FD592E /* DFGLiveCatchVariablePreservationPhase.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 79C4B15B1BA2158F00FD592E /* DFGLiveCatchVariablePreservationPhase.cpp */; };

  • trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp

    r201445 r201495  
    219219        break;
    220220    case ScopedArgumentsType:
    221         length =jsCast<ScopedArguments*>(cell)->length(callFrame);
     221        length = jsCast<ScopedArguments*>(cell)->length(callFrame);
    222222        break;
    223223    case StringType:
    … …  
    227227        ASSERT(arguments.isObject());
    228228        length = getLength(callFrame, jsCast<JSObject*>(cell));
     229        if (UNLIKELY(callFrame->hadException()))
     230            return 0;
    229231        break;
    230232    }
     233
    231234   
    232235    if (length >= firstVarArgOffset)

  • trunk/Source/JavaScriptCore/runtime/InternalFunction.cpp

    r200430 r201495  
    110110
    111111            // Note, Reflect.construct might cause the profile to churn but we don't care.
    112             JSObject* prototype = jsDynamicCast<JSObject*>(newTarget.get(exec, exec->propertyNames().prototype));
    113             if (exec->hadException())
     112            JSValue prototypeValue = newTarget.get(exec, exec->propertyNames().prototype);
     113            if (UNLIKELY(vm.exception()))
    114114                return nullptr;
    115             if (prototype)
     115            if (JSObject* prototype = jsDynamicCast<JSObject*>(prototypeValue))
    116116                return targetFunction->rareData(vm)->createInternalFunctionAllocationStructureFromBase(vm, prototype, baseClass);
    117117        } else {
    118             JSObject* prototype = jsDynamicCast<JSObject*>(newTarget.get(exec, exec->propertyNames().prototype));
    119             if (exec->hadException())
     118            JSValue prototypeValue = newTarget.get(exec, exec->propertyNames().prototype);
     119            if (UNLIKELY(vm.exception()))
    120120                return nullptr;
    121             if (prototype) {
     121            if (JSObject* prototype = jsDynamicCast<JSObject*>(prototypeValue)) {
    122122                // This only happens if someone Reflect.constructs our builtin constructor with another builtin constructor as the new.target.
    123123                // Thus, we don't care about the cost of looking up the structure from our hash table every time.

  • trunk/Source/JavaScriptCore/runtime/JSArray.h

    r201049 r201495  
    353353    if (isJSArray(obj))
    354354        return jsCast<JSArray*>(obj)->length();
    355     return obj->get(exec, exec->propertyNames().length).toUInt32(exec);
     355
     356    VM& vm = exec->vm();
     357    JSValue lengthValue = obj->get(exec, vm.propertyNames->length);
     358    if (UNLIKELY(vm.exception()))
     359        return UINT_MAX;
     360    return lengthValue.toUInt32(exec);
    356361}
    357362

  • trunk/Source/JavaScriptCore/runtime/ObjectPrototype.cpp

    r197794 r201495  
    283283        if (thisObject->getPropertySlot(exec, toStringTagSymbol, toStringTagSlot)) {
    284284            JSValue stringTag = toStringTagSlot.getValue(exec, toStringTagSymbol);
     285            if (UNLIKELY(vm.exception()))
     286                return JSValue::encode(JSValue());
    285287            if (stringTag.isString()) {
    286288                JSRopeString::RopeBuilder ropeBuilder(vm);

  • trunk/Source/JavaScriptCore/runtime/ProxyObject.cpp

    r201322 r201495  
    3535#include "StructureInlines.h"
    3636
     37// Note that this file is compile with -fno-optimize-sibling-calls because we rely on the machine stack
     38// growing larger for throwing OOM errors for when we have an effectively cyclic prototype chain.
     39
    3740namespace JSC {
    3841
    … …  
    9699{
    97100    VM& vm = exec->vm();
    98     if (!vm.isSafeToRecurse()) {
     101    if (UNLIKELY(!vm.isSafeToRecurse())) {
    99102        throwStackOverflowError(exec);
    100103        return JSValue::encode(JSValue());
    … …  
    156159{
    157160    VM& vm = exec->vm();
     161    if (UNLIKELY(!vm.isSafeToRecurse())) {
     162        throwStackOverflowError(exec);
     163        return false;
     164    }
    158165    JSObject* target = this->target();
    159166
    … …  
    258265{
    259266    VM& vm = exec->vm();
     267    if (UNLIKELY(!vm.isSafeToRecurse())) {
     268        throwStackOverflowError(exec);
     269        return false;
     270    }
    260271    JSObject* target = this->target();
    261272    slot.setValue(this, None, jsUndefined()); // Nobody should rely on our value, but be safe and protect against any bad actors reading our value.
    … …  
    319330bool ProxyObject::getOwnPropertySlotCommon(ExecState* exec, PropertyName propertyName, PropertySlot& slot)
    320331{
     332    if (UNLIKELY(!exec->vm().isSafeToRecurse())) {
     333        throwStackOverflowError(exec);
     334        return false;
     335    }
    321336    slot.disableCaching();
    322337    slot.setIsTaintedByProxy();
    … …  
    354369{
    355370    VM& vm = exec->vm();
    356     if (!vm.isSafeToRecurse()) {
     371    if (UNLIKELY(!vm.isSafeToRecurse())) {
    357372        throwStackOverflowError(exec);
    358373        return false;
    … …  
    446461{
    447462    VM& vm = exec->vm();
     463    if (UNLIKELY(!vm.isSafeToRecurse())) {
     464        throwStackOverflowError(exec);
     465        return JSValue::encode(JSValue());
     466    }
    448467    ProxyObject* proxy = jsCast<ProxyObject*>(exec->callee());
    449468    JSValue handlerValue = proxy->handler();
    … …  
    491510{
    492511    VM& vm = exec->vm();
     512    if (UNLIKELY(!vm.isSafeToRecurse())) {
     513        throwStackOverflowError(exec);
     514        return JSValue::encode(JSValue());
     515    }
    493516    ProxyObject* proxy = jsCast<ProxyObject*>(exec->callee());
    494517    JSValue handlerValue = proxy->handler();
    … …  
    542565{
    543566    VM& vm = exec->vm();
     567    if (UNLIKELY(!vm.isSafeToRecurse())) {
     568        throwStackOverflowError(exec);
     569        return false;
     570    }
    544571
    545572    if (vm.propertyNames->isPrivateName(Identifier::fromUid(&vm, propertyName.uid())))
    … …  
    614641{
    615642    VM& vm = exec->vm();
     643    if (UNLIKELY(!vm.isSafeToRecurse())) {
     644        throwStackOverflowError(exec);
     645        return false;
     646    }
    616647
    617648    JSValue handlerValue = this->handler();
    … …  
    662693{
    663694    VM& vm = exec->vm();
     695    if (UNLIKELY(!vm.isSafeToRecurse())) {
     696        throwStackOverflowError(exec);
     697        return false;
     698    }
    664699
    665700    JSValue handlerValue = this->handler();
    … …  
    716751{
    717752    VM& vm = exec->vm();
     753    if (UNLIKELY(!vm.isSafeToRecurse())) {
     754        throwStackOverflowError(exec);
     755        return false;
     756    }
    718757
    719758    JSObject* target = this->target();
    … …  
    809848{
    810849    VM& vm = exec->vm();
     850    if (UNLIKELY(!vm.isSafeToRecurse())) {
     851        throwStackOverflowError(exec);
     852        return;
     853    }
    811854    JSValue handlerValue = this->handler();
    812855    if (handlerValue.isNull()) {
    … …  
    948991}
    949992
     993void ProxyObject::getPropertyNames(JSObject* object, ExecState* exec, PropertyNameArray& propertyNameArray, EnumerationMode enumerationMode)
     994{
     995    JSObject::getPropertyNames(object, exec, propertyNameArray, enumerationMode);
     996}
     997
    950998void ProxyObject::getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode)
    951999{
    … …  
    9691017
    9701018    VM& vm = exec->vm();
     1019    if (UNLIKELY(!vm.isSafeToRecurse())) {
     1020        throwStackOverflowError(exec);
     1021        return false;
     1022    }
    9711023
    9721024    JSValue handlerValue = this->handler();
    … …  
    10291081{
    10301082    VM& vm = exec->vm();
     1083    if (UNLIKELY(!vm.isSafeToRecurse())) {
     1084        throwStackOverflowError(exec);
     1085        return JSValue();
     1086    }
    10311087
    10321088    JSValue handlerValue = this->handler();

  • trunk/Source/JavaScriptCore/runtime/ProxyObject.h

    r198023 r201495  
    3737    typedef JSNonFinalObject Base;
    3838
    39     // We lie an say we override getPropertyNames() because it prevents
    40     // property name enumeration caching.
    4139    const static unsigned StructureFlags = Base::StructureFlags | OverridesGetOwnPropertySlot | TypeOfShouldCallGetCallData | InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero | OverridesGetPropertyNames | ProhibitsPropertyCaching;
    4240
    … …  
    8886    static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
    8987    static void getOwnPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
     88    static void getPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
    9089    static NO_RETURN_DUE_TO_CRASH void getOwnNonIndexPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
    9190    static NO_RETURN_DUE_TO_CRASH void getStructurePropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode);
Note: See TracChangeset for help on using the changeset viewer.