Changeset 248508 in webkit


Ignore:
Timestamp:
Aug 10, 2019 8:02:10 PM (5 years ago)
Author:
bshafiei@apple.com
Message:

Cherry-pick r248494. rdar://problem/54171879

Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
​https://bugs.webkit.org/show_bug.cgi?id=199864

Reviewed by Saam Barati.

Source/JavaScriptCore:

Our JSObject::put implementation is not correct in term of the spec. Our Put? implementation is something like this.

JSObject::put(object):

if (can-do-fast-path(object))

return fast-path(object);

slow-path
do {

object-put-check-and-setter-calls(object); (1)
object = object->prototype;

} while (is-object(object));
return do-put(object);

Since JSObject::put is registered in the methodTable, the derived classes can override it. Some of classes are adding
extra checks to this put.

Derived::put(object):

if (do-extra-check(object))

fail

return JSObject::put(object)

The problem is that Derived::put is only called when the |this| object is the Derived class. When traversing Prototype? in
JSObject::put, at (1), we do not perform the extra checks added in Derived::put even if object is Derived one. This means that
we skip the check.

Currently, JSObject::put and WebCore checking mechanism are broken. JSObject::put should call getOwnPropertySlot at (1) to
perform the additional checks. This behavior is matching against the spec. However, currently, our JSObject::getOwnPropertySlot
does not propagate setter information. This is required to cache cacheable Put? at (1) for CustomValue, CustomAccessor, and
Accessors. We also need to reconsider how to integrate static property setters to this mechanism. So, basically, this involves
large refactoring to renew our JSObject::put and JSObject::getOwnPropertySlot.

To work-around for now, we add a new TypeInfo flag, HasPutPropertySecurityCheck . And adding this flag to DOM objects
that implements the addition checks. We also add doPutPropertySecurityCheck method hook to perform the check in JSObject.
When we found this flag at (1), we perform doPutPropertySecurityCheck to properly perform the checks.

Since our JSObject::put code is old and it does not match against the spec now, we should refactor it largely. This is tracked separately in [1].

[1]: ​https://bugs.webkit.org/show_bug.cgi?id=200562

  • runtime/ClassInfo.h:
  • runtime/JSCJSValue.cpp: (JSC::JSValue::putToPrimitive):
  • runtime/JSCell.cpp: (JSC::JSCell::doPutPropertySecurityCheck):
  • runtime/JSCell.h:
  • runtime/JSObject.cpp: (JSC::JSObject::putInlineSlow): (JSC::JSObject::getOwnPropertyDescriptor):
  • runtime/JSObject.h: (JSC::JSObject::doPutPropertySecurityCheck):
  • runtime/JSTypeInfo.h: (JSC::TypeInfo::hasPutPropertySecurityCheck const):

Source/WebCore:

Test: http/tests/security/cross-frame-access-object-put-optimization.html

  • bindings/js/JSDOMWindowCustom.cpp: (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
  • bindings/js/JSLocationCustom.cpp: (WebCore::JSLocation::doPutPropertySecurityCheck):
  • bindings/scripts/CodeGeneratorJS.pm: (GenerateHeader):
  • bindings/scripts/test/JS/JSTestActiveDOMObject.h:

LayoutTests:

  • http/tests/security/cross-frame-access-object-put-optimization-expected.txt: Added.
  • http/tests/security/cross-frame-access-object-put-optimization.html: Added.
  • http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html: Added.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@248494 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-608.1-branch
Files:
3 added
14 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608.1-branch/LayoutTests/ChangeLog

    r248507 r248508  
     12019-08-10  Babak Shafiei  <bshafiei@apple.com>
     2
     3        Cherry-pick r248494. rdar://problem/54171879
     4
     5    Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
     6    https://bugs.webkit.org/show_bug.cgi?id=199864
     7   
     8    Reviewed by Saam Barati.
     9   
     10    Source/JavaScriptCore:
     11   
     12    Our JSObject::put implementation is not correct in term of the spec. Our [[Put]] implementation is something like this.
     13   
     14        JSObject::put(object):
     15            if (can-do-fast-path(object))
     16                return fast-path(object);
     17            // slow-path
     18            do {
     19                object-put-check-and-setter-calls(object); // (1)
     20                object = object->prototype;
     21            } while (is-object(object));
     22            return do-put(object);
     23   
     24    Since JSObject::put is registered in the methodTable, the derived classes can override it. Some of classes are adding
     25    extra checks to this put.
     26   
     27        Derived::put(object):
     28            if (do-extra-check(object))
     29                fail
     30            return JSObject::put(object)
     31   
     32    The problem is that Derived::put is only called when the |this| object is the Derived class. When traversing [[Prototype]] in
     33    JSObject::put, at (1), we do not perform the extra checks added in Derived::put even if `object` is Derived one. This means that
     34    we skip the check.
     35   
     36    Currently, JSObject::put and WebCore checking mechanism are broken. JSObject::put should call getOwnPropertySlot at (1) to
     37    perform the additional checks. This behavior is matching against the spec. However, currently, our JSObject::getOwnPropertySlot
     38    does not propagate setter information. This is required to cache cacheable [[Put]] at (1) for CustomValue, CustomAccessor, and
     39    Accessors. We also need to reconsider how to integrate static property setters to this mechanism. So, basically, this involves
     40    large refactoring to renew our JSObject::put and JSObject::getOwnPropertySlot.
     41   
     42    To work-around for now, we add a new TypeInfo flag, HasPutPropertySecurityCheck . And adding this flag to DOM objects
     43    that implements the addition checks. We also add doPutPropertySecurityCheck method hook to perform the check in JSObject.
     44    When we found this flag at (1), we perform doPutPropertySecurityCheck to properly perform the checks.
     45   
     46    Since our JSObject::put code is old and it does not match against the spec now, we should refactor it largely. This is tracked separately in [1].
     47   
     48    [1]: https://bugs.webkit.org/show_bug.cgi?id=200562
     49   
     50    * runtime/ClassInfo.h:
     51    * runtime/JSCJSValue.cpp:
     52    (JSC::JSValue::putToPrimitive):
     53    * runtime/JSCell.cpp:
     54    (JSC::JSCell::doPutPropertySecurityCheck):
     55    * runtime/JSCell.h:
     56    * runtime/JSObject.cpp:
     57    (JSC::JSObject::putInlineSlow):
     58    (JSC::JSObject::getOwnPropertyDescriptor):
     59    * runtime/JSObject.h:
     60    (JSC::JSObject::doPutPropertySecurityCheck):
     61    * runtime/JSTypeInfo.h:
     62    (JSC::TypeInfo::hasPutPropertySecurityCheck const):
     63   
     64    Source/WebCore:
     65   
     66    Test: http/tests/security/cross-frame-access-object-put-optimization.html
     67   
     68    * bindings/js/JSDOMWindowCustom.cpp:
     69    (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
     70    * bindings/js/JSLocationCustom.cpp:
     71    (WebCore::JSLocation::doPutPropertySecurityCheck):
     72    * bindings/scripts/CodeGeneratorJS.pm:
     73    (GenerateHeader):
     74    * bindings/scripts/test/JS/JSTestActiveDOMObject.h:
     75   
     76    LayoutTests:
     77   
     78    * http/tests/security/cross-frame-access-object-put-optimization-expected.txt: Added.
     79    * http/tests/security/cross-frame-access-object-put-optimization.html: Added.
     80    * http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html: Added.
     81   
     82    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248494 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     83
     84    2019-08-09  Yusuke Suzuki  <ysuzuki@apple.com>
     85
     86            Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
     87            https://bugs.webkit.org/show_bug.cgi?id=199864
     88
     89            Reviewed by Saam Barati.
     90
     91            * http/tests/security/cross-frame-access-object-put-optimization-expected.txt: Added.
     92            * http/tests/security/cross-frame-access-object-put-optimization.html: Added.
     93            * http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html: Added.
     94
    1952019-08-10  Babak Shafiei  <bshafiei@apple.com>
    296

  • branches/safari-608.1-branch/Source/JavaScriptCore/ChangeLog

    r247794 r248508  
     12019-08-10  Babak Shafiei  <bshafiei@apple.com>
     2
     3        Cherry-pick r248494. rdar://problem/54171879
     4
     5    Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
     6    https://bugs.webkit.org/show_bug.cgi?id=199864
     7   
     8    Reviewed by Saam Barati.
     9   
     10    Source/JavaScriptCore:
     11   
     12    Our JSObject::put implementation is not correct in term of the spec. Our [[Put]] implementation is something like this.
     13   
     14        JSObject::put(object):
     15            if (can-do-fast-path(object))
     16                return fast-path(object);
     17            // slow-path
     18            do {
     19                object-put-check-and-setter-calls(object); // (1)
     20                object = object->prototype;
     21            } while (is-object(object));
     22            return do-put(object);
     23   
     24    Since JSObject::put is registered in the methodTable, the derived classes can override it. Some of classes are adding
     25    extra checks to this put.
     26   
     27        Derived::put(object):
     28            if (do-extra-check(object))
     29                fail
     30            return JSObject::put(object)
     31   
     32    The problem is that Derived::put is only called when the |this| object is the Derived class. When traversing [[Prototype]] in
     33    JSObject::put, at (1), we do not perform the extra checks added in Derived::put even if `object` is Derived one. This means that
     34    we skip the check.
     35   
     36    Currently, JSObject::put and WebCore checking mechanism are broken. JSObject::put should call getOwnPropertySlot at (1) to
     37    perform the additional checks. This behavior is matching against the spec. However, currently, our JSObject::getOwnPropertySlot
     38    does not propagate setter information. This is required to cache cacheable [[Put]] at (1) for CustomValue, CustomAccessor, and
     39    Accessors. We also need to reconsider how to integrate static property setters to this mechanism. So, basically, this involves
     40    large refactoring to renew our JSObject::put and JSObject::getOwnPropertySlot.
     41   
     42    To work-around for now, we add a new TypeInfo flag, HasPutPropertySecurityCheck . And adding this flag to DOM objects
     43    that implements the addition checks. We also add doPutPropertySecurityCheck method hook to perform the check in JSObject.
     44    When we found this flag at (1), we perform doPutPropertySecurityCheck to properly perform the checks.
     45   
     46    Since our JSObject::put code is old and it does not match against the spec now, we should refactor it largely. This is tracked separately in [1].
     47   
     48    [1]: https://bugs.webkit.org/show_bug.cgi?id=200562
     49   
     50    * runtime/ClassInfo.h:
     51    * runtime/JSCJSValue.cpp:
     52    (JSC::JSValue::putToPrimitive):
     53    * runtime/JSCell.cpp:
     54    (JSC::JSCell::doPutPropertySecurityCheck):
     55    * runtime/JSCell.h:
     56    * runtime/JSObject.cpp:
     57    (JSC::JSObject::putInlineSlow):
     58    (JSC::JSObject::getOwnPropertyDescriptor):
     59    * runtime/JSObject.h:
     60    (JSC::JSObject::doPutPropertySecurityCheck):
     61    * runtime/JSTypeInfo.h:
     62    (JSC::TypeInfo::hasPutPropertySecurityCheck const):
     63   
     64    Source/WebCore:
     65   
     66    Test: http/tests/security/cross-frame-access-object-put-optimization.html
     67   
     68    * bindings/js/JSDOMWindowCustom.cpp:
     69    (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
     70    * bindings/js/JSLocationCustom.cpp:
     71    (WebCore::JSLocation::doPutPropertySecurityCheck):
     72    * bindings/scripts/CodeGeneratorJS.pm:
     73    (GenerateHeader):
     74    * bindings/scripts/test/JS/JSTestActiveDOMObject.h:
     75   
     76    LayoutTests:
     77   
     78    * http/tests/security/cross-frame-access-object-put-optimization-expected.txt: Added.
     79    * http/tests/security/cross-frame-access-object-put-optimization.html: Added.
     80    * http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html: Added.
     81   
     82    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248494 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     83
     84    2019-08-09  Yusuke Suzuki  <ysuzuki@apple.com>
     85
     86            Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
     87            https://bugs.webkit.org/show_bug.cgi?id=199864
     88
     89            Reviewed by Saam Barati.
     90
     91            Our JSObject::put implementation is not correct in term of the spec. Our [[Put]] implementation is something like this.
     92
     93                JSObject::put(object):
     94                    if (can-do-fast-path(object))
     95                        return fast-path(object);
     96                    // slow-path
     97                    do {
     98                        object-put-check-and-setter-calls(object); // (1)
     99                        object = object->prototype;
     100                    } while (is-object(object));
     101                    return do-put(object);
     102
     103            Since JSObject::put is registered in the methodTable, the derived classes can override it. Some of classes are adding
     104            extra checks to this put.
     105
     106                Derived::put(object):
     107                    if (do-extra-check(object))
     108                        fail
     109                    return JSObject::put(object)
     110
     111            The problem is that Derived::put is only called when the |this| object is the Derived class. When traversing [[Prototype]] in
     112            JSObject::put, at (1), we do not perform the extra checks added in Derived::put even if `object` is Derived one. This means that
     113            we skip the check.
     114
     115            Currently, JSObject::put and WebCore checking mechanism are broken. JSObject::put should call getOwnPropertySlot at (1) to
     116            perform the additional checks. This behavior is matching against the spec. However, currently, our JSObject::getOwnPropertySlot
     117            does not propagate setter information. This is required to cache cacheable [[Put]] at (1) for CustomValue, CustomAccessor, and
     118            Accessors. We also need to reconsider how to integrate static property setters to this mechanism. So, basically, this involves
     119            large refactoring to renew our JSObject::put and JSObject::getOwnPropertySlot.
     120
     121            To work-around for now, we add a new TypeInfo flag, HasPutPropertySecurityCheck . And adding this flag to DOM objects
     122            that implements the addition checks. We also add doPutPropertySecurityCheck method hook to perform the check in JSObject.
     123            When we found this flag at (1), we perform doPutPropertySecurityCheck to properly perform the checks.
     124
     125            Since our JSObject::put code is old and it does not match against the spec now, we should refactor it largely. This is tracked separately in [1].
     126
     127            [1]: https://bugs.webkit.org/show_bug.cgi?id=200562
     128
     129            * runtime/ClassInfo.h:
     130            * runtime/JSCJSValue.cpp:
     131            (JSC::JSValue::putToPrimitive):
     132            * runtime/JSCell.cpp:
     133            (JSC::JSCell::doPutPropertySecurityCheck):
     134            * runtime/JSCell.h:
     135            * runtime/JSObject.cpp:
     136            (JSC::JSObject::putInlineSlow):
     137            (JSC::JSObject::getOwnPropertyDescriptor):
     138            * runtime/JSObject.h:
     139            (JSC::JSObject::doPutPropertySecurityCheck):
     140            * runtime/JSTypeInfo.h:
     141            (JSC::TypeInfo::hasPutPropertySecurityCheck const):
     142
    11432019-07-24  Alan Coon  <alancoon@apple.com>
    2144

  • branches/safari-608.1-branch/Source/JavaScriptCore/runtime/ClassInfo.h

    r243254 r248508  
    7979    using GetOwnPropertySlotByIndexFunctionPtr = bool (*)(JSObject*, ExecState*, unsigned, PropertySlot&);
    8080    GetOwnPropertySlotByIndexFunctionPtr WTF_METHOD_TABLE_ENTRY(getOwnPropertySlotByIndex);
     81
     82    using DoPutPropertySecurityCheckFunctionPtr = void (*)(JSObject*, ExecState*, PropertyName, PutPropertySlot&);
     83    DoPutPropertySecurityCheckFunctionPtr METHOD_TABLE_ENTRY(doPutPropertySecurityCheck);
    8184
    8285    using ToThisFunctionPtr = JSValue (*)(JSCell*, ExecState*, ECMAMode);
    … …  
    168171        &ClassName::getOwnPropertySlot, \
    169172        &ClassName::getOwnPropertySlotByIndex, \
     173        &ClassName::doPutPropertySecurityCheck, \
    170174        &ClassName::toThis, \
    171175        &ClassName::defaultValue, \

  • branches/safari-608.1-branch/Source/JavaScriptCore/runtime/JSCJSValue.cpp

    r246490 r248508  
    163163    JSValue prototype;
    164164    if (propertyName != vm.propertyNames->underscoreProto) {
    165         for (; !obj->structure(vm)->hasReadOnlyOrGetterSetterPropertiesExcludingProto(); obj = asObject(prototype)) {
     165        while (true) {
     166            Structure* structure = obj->structure(vm);
     167            if (structure->hasReadOnlyOrGetterSetterPropertiesExcludingProto() || structure->typeInfo().hasPutPropertySecurityCheck())
     168                break;
    166169            prototype = obj->getPrototype(vm, exec);
    167170            RETURN_IF_EXCEPTION(scope, false);
    … …  
    169172            if (prototype.isNull())
    170173                return typeError(exec, scope, slot.isStrictMode(), ReadonlyPropertyWriteError);
     174            obj = asObject(prototype);
    171175        }
    172176    }
    173177
    174178    for (; ; obj = asObject(prototype)) {
     179        Structure* structure = obj->structure(vm);
     180        if (UNLIKELY(structure->typeInfo().hasPutPropertySecurityCheck())) {
     181            obj->methodTable(vm)->doPutPropertySecurityCheck(obj, exec, propertyName, slot);
     182            RETURN_IF_EXCEPTION(scope, false);
     183        }
    175184        unsigned attributes;
    176         PropertyOffset offset = obj->structure(vm)->get(vm, propertyName, attributes);
     185        PropertyOffset offset = structure->get(vm, propertyName, attributes);
    177186        if (offset != invalidOffset) {
    178187            if (attributes & PropertyAttribute::ReadOnly)

  • branches/safari-608.1-branch/Source/JavaScriptCore/runtime/JSCell.cpp

    r233765 r248508  
    213213}
    214214
     215void JSCell::doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&)
     216{
     217    RELEASE_ASSERT_NOT_REACHED();
     218}
     219
    215220void JSCell::getOwnPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode)
    216221{

  • branches/safari-608.1-branch/Source/JavaScriptCore/runtime/JSCell.h

    r240965 r248508  
    266266    static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
    267267    static bool getOwnPropertySlotByIndex(JSObject*, ExecState*, unsigned propertyName, PropertySlot&);
     268    static NO_RETURN_DUE_TO_CRASH void doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&);
    268269
    269270private:

  • branches/safari-608.1-branch/Source/JavaScriptCore/runtime/JSObject.cpp

    r246084 r248508  
    791791    JSObject* obj = this;
    792792    for (;;) {
     793        Structure* structure = obj->structure(vm);
     794        if (UNLIKELY(structure->typeInfo().hasPutPropertySecurityCheck())) {
     795            obj->methodTable(vm)->doPutPropertySecurityCheck(obj, exec, propertyName, slot);
     796            RETURN_IF_EXCEPTION(scope, false);
     797        }
    793798        unsigned attributes;
    794         PropertyOffset offset = obj->structure(vm)->get(vm, propertyName, attributes);
     799        PropertyOffset offset = structure->get(vm, propertyName, attributes);
    795800        if (isValidOffset(offset)) {
    796801            if (attributes & PropertyAttribute::ReadOnly) {
    … …  
    802807            if (gs.isGetterSetter()) {
    803808                // We need to make sure that we decide to cache this property before we potentially execute aribitrary JS.
    804                 if (!structure(vm)->isDictionary())
     809                if (!this->structure(vm)->isDictionary())
    805810                    slot.setCacheableSetter(obj, offset);
    806811
    … …  
    34693474        return false;
    34703475
     3476
     3477    // FIXME: https://bugs.webkit.org/show_bug.cgi?id=200560
     3478    // This breaks the assumption that getOwnPropertySlot should return "own" property.
     3479    // We should fix DebuggerScope, ProxyObject etc. to remove this.
     3480    //
    34713481    // DebuggerScope::getOwnPropertySlot() (and possibly others) may return attributes from the prototype chain
    34723482    // but getOwnPropertyDescriptor() should only work for 'own' properties so we exit early if we detect that

  • branches/safari-608.1-branch/Source/JavaScriptCore/runtime/JSObject.h

    r247386 r248508  
    169169    JS_EXPORT_PRIVATE static bool getOwnPropertySlotByIndex(JSObject*, ExecState*, unsigned propertyName, PropertySlot&);
    170170    bool getOwnPropertySlotInline(ExecState*, PropertyName, PropertySlot&);
     171    static void doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&);
    171172
    172173    // The key difference between this and getOwnPropertySlot is that getOwnPropertySlot
    … …  
    14051406}
    14061407
     1408ALWAYS_INLINE void JSObject::doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&)
     1409{
     1410}
     1411
    14071412// It may seem crazy to inline a function this large but it makes a big difference
    14081413// since this is function very hot in variable lookup

  • branches/safari-608.1-branch/Source/JavaScriptCore/runtime/JSTypeInfo.h

    r240951 r248508  
    5757static const unsigned InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero = 1 << 15;
    5858static const unsigned StructureIsImmortal = 1 << 16;
     59static const unsigned HasPutPropertySecurityCheck = 1 << 17;
    5960
    6061class TypeInfo {
    … …  
    9798    bool getOwnPropertySlotIsImpure() const { return isSetOnFlags2(GetOwnPropertySlotIsImpure); }
    9899    bool getOwnPropertySlotIsImpureForPropertyAbsence() const { return isSetOnFlags2(GetOwnPropertySlotIsImpureForPropertyAbsence); }
     100    bool hasPutPropertySecurityCheck() const { return isSetOnFlags2(HasPutPropertySecurityCheck); }
    99101    bool newImpurePropertyFiresWatchpoints() const { return isSetOnFlags2(NewImpurePropertyFiresWatchpoints); }
    100102    bool isImmutablePrototypeExoticObject() const { return isSetOnFlags2(IsImmutablePrototypeExoticObject); }

  • branches/safari-608.1-branch/Source/WebCore/ChangeLog

    r248507 r248508  
     12019-08-10  Babak Shafiei  <bshafiei@apple.com>
     2
     3        Cherry-pick r248494. rdar://problem/54171879
     4
     5    Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
     6    https://bugs.webkit.org/show_bug.cgi?id=199864
     7   
     8    Reviewed by Saam Barati.
     9   
     10    Source/JavaScriptCore:
     11   
     12    Our JSObject::put implementation is not correct in term of the spec. Our [[Put]] implementation is something like this.
     13   
     14        JSObject::put(object):
     15            if (can-do-fast-path(object))
     16                return fast-path(object);
     17            // slow-path
     18            do {
     19                object-put-check-and-setter-calls(object); // (1)
     20                object = object->prototype;
     21            } while (is-object(object));
     22            return do-put(object);
     23   
     24    Since JSObject::put is registered in the methodTable, the derived classes can override it. Some of classes are adding
     25    extra checks to this put.
     26   
     27        Derived::put(object):
     28            if (do-extra-check(object))
     29                fail
     30            return JSObject::put(object)
     31   
     32    The problem is that Derived::put is only called when the |this| object is the Derived class. When traversing [[Prototype]] in
     33    JSObject::put, at (1), we do not perform the extra checks added in Derived::put even if `object` is Derived one. This means that
     34    we skip the check.
     35   
     36    Currently, JSObject::put and WebCore checking mechanism are broken. JSObject::put should call getOwnPropertySlot at (1) to
     37    perform the additional checks. This behavior is matching against the spec. However, currently, our JSObject::getOwnPropertySlot
     38    does not propagate setter information. This is required to cache cacheable [[Put]] at (1) for CustomValue, CustomAccessor, and
     39    Accessors. We also need to reconsider how to integrate static property setters to this mechanism. So, basically, this involves
     40    large refactoring to renew our JSObject::put and JSObject::getOwnPropertySlot.
     41   
     42    To work-around for now, we add a new TypeInfo flag, HasPutPropertySecurityCheck . And adding this flag to DOM objects
     43    that implements the addition checks. We also add doPutPropertySecurityCheck method hook to perform the check in JSObject.
     44    When we found this flag at (1), we perform doPutPropertySecurityCheck to properly perform the checks.
     45   
     46    Since our JSObject::put code is old and it does not match against the spec now, we should refactor it largely. This is tracked separately in [1].
     47   
     48    [1]: https://bugs.webkit.org/show_bug.cgi?id=200562
     49   
     50    * runtime/ClassInfo.h:
     51    * runtime/JSCJSValue.cpp:
     52    (JSC::JSValue::putToPrimitive):
     53    * runtime/JSCell.cpp:
     54    (JSC::JSCell::doPutPropertySecurityCheck):
     55    * runtime/JSCell.h:
     56    * runtime/JSObject.cpp:
     57    (JSC::JSObject::putInlineSlow):
     58    (JSC::JSObject::getOwnPropertyDescriptor):
     59    * runtime/JSObject.h:
     60    (JSC::JSObject::doPutPropertySecurityCheck):
     61    * runtime/JSTypeInfo.h:
     62    (JSC::TypeInfo::hasPutPropertySecurityCheck const):
     63   
     64    Source/WebCore:
     65   
     66    Test: http/tests/security/cross-frame-access-object-put-optimization.html
     67   
     68    * bindings/js/JSDOMWindowCustom.cpp:
     69    (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
     70    * bindings/js/JSLocationCustom.cpp:
     71    (WebCore::JSLocation::doPutPropertySecurityCheck):
     72    * bindings/scripts/CodeGeneratorJS.pm:
     73    (GenerateHeader):
     74    * bindings/scripts/test/JS/JSTestActiveDOMObject.h:
     75   
     76    LayoutTests:
     77   
     78    * http/tests/security/cross-frame-access-object-put-optimization-expected.txt: Added.
     79    * http/tests/security/cross-frame-access-object-put-optimization.html: Added.
     80    * http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html: Added.
     81   
     82    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@248494 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     83
     84    2019-08-09  Yusuke Suzuki  <ysuzuki@apple.com>
     85
     86            Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
     87            https://bugs.webkit.org/show_bug.cgi?id=199864
     88
     89            Reviewed by Saam Barati.
     90
     91            Test: http/tests/security/cross-frame-access-object-put-optimization.html
     92
     93            * bindings/js/JSDOMWindowCustom.cpp:
     94            (WebCore::JSDOMWindow::doPutPropertySecurityCheck):
     95            * bindings/js/JSLocationCustom.cpp:
     96            (WebCore::JSLocation::doPutPropertySecurityCheck):
     97            * bindings/scripts/CodeGeneratorJS.pm:
     98            (GenerateHeader):
     99            * bindings/scripts/test/JS/JSTestActiveDOMObject.h:
     100
    11012019-08-10  Babak Shafiei  <bshafiei@apple.com>
    2102

  • branches/safari-608.1-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp

    r247576 r248508  
    265265}
    266266
     267void JSDOMWindow::doPutPropertySecurityCheck(JSObject* cell, ExecState* state, PropertyName propertyName, PutPropertySlot&)
     268{
     269    VM& vm = state->vm();
     270    auto scope = DECLARE_THROW_SCOPE(vm);
     271
     272    auto* thisObject = jsCast<JSDOMWindow*>(cell);
     273    if (!thisObject->wrapped().frame())
     274        return;
     275
     276    String errorMessage;
     277    if (!BindingSecurity::shouldAllowAccessToDOMWindow(*state, thisObject->wrapped(), errorMessage)) {
     278        // We only allow setting "location" attribute cross-origin.
     279        if (propertyName == static_cast<JSVMClientData*>(vm.clientData)->builtinNames().locationPublicName())
     280            return;
     281        throwSecurityError(*state, scope, errorMessage);
     282        return;
     283    }
     284}
     285
    267286bool JSDOMWindow::put(JSCell* cell, ExecState* state, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
    268287{

  • branches/safari-608.1-branch/Source/WebCore/bindings/js/JSLocationCustom.cpp

    r245249 r248508  
    127127}
    128128
     129void JSLocation::doPutPropertySecurityCheck(JSObject* object, ExecState* state, PropertyName propertyName, PutPropertySlot&)
     130{
     131    auto* thisObject = jsCast<JSLocation*>(object);
     132    ASSERT_GC_OBJECT_INHERITS(thisObject, info());
     133
     134    VM& vm = state->vm();
     135
     136    // Always allow assigning to the whole location.
     137    // However, alllowing assigning of pieces might inadvertently disclose parts of the original location.
     138    // So fall through to the access check for those.
     139    if (propertyName == static_cast<JSVMClientData*>(vm.clientData)->builtinNames().hrefPublicName())
     140        return;
     141
     142    // Block access and throw if there is a security error.
     143    BindingSecurity::shouldAllowAccessToDOMWindow(state, thisObject->wrapped().window(), ThrowSecurityError);
     144}
     145
    129146bool JSLocation::put(JSCell* cell, ExecState* state, PropertyName propertyName, JSValue value, PutPropertySlot& putPropertySlot)
    130147{

  • branches/safari-608.1-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r247278 r248508  
    26392639        push(@headerContent, "    static bool getOwnPropertySlotByIndex(JSC::JSObject*, JSC::ExecState*, unsigned propertyName, JSC::PropertySlot&);\n");
    26402640        $structureFlags{"JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero"} = 1;
     2641    }
     2642
     2643    if ($interface->extendedAttributes->{CheckSecurity}) {
     2644        push(@headerContent, "    static void doPutPropertySecurityCheck(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PutPropertySlot&);\n");
     2645        $structureFlags{"JSC::HasPutPropertySecurityCheck"} = 1;
    26412646    }
    26422647   

  • branches/safari-608.1-branch/Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h

    r239191 r248508  
    4040    static JSC::JSObject* prototype(JSC::VM&, JSDOMGlobalObject&);
    4141    static TestActiveDOMObject* toWrapped(JSC::VM&, JSC::JSValue);
     42    static void doPutPropertySecurityCheck(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PutPropertySlot&);
    4243    static void destroy(JSC::JSCell*);
    4344
    … …  
    5253    static void heapSnapshot(JSCell*, JSC::HeapSnapshotBuilder&);
    5354public:
    54     static const unsigned StructureFlags = Base::StructureFlags | JSC::HasStaticPropertyTable;
     55    static const unsigned StructureFlags = Base::StructureFlags | JSC::HasPutPropertySecurityCheck | JSC::HasStaticPropertyTable;
    5556protected:
    5657    JSTestActiveDOMObject(JSC::Structure*, JSDOMGlobalObject&, Ref<TestActiveDOMObject>&&);
Note: See TracChangeset for help on using the changeset viewer.