Diff
Modified: branches/safari-608-branch/LayoutTests/ChangeLog (248576 => 248577)
--- branches/safari-608-branch/LayoutTests/ChangeLog 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/LayoutTests/ChangeLog 2019-08-12 23:42:48 UTC (rev 248577)
@@ -1,5 +1,99 @@
2019-08-12 Alan Coon <[email protected]>
+ Cherry-pick r248494. rdar://problem/54171876
+
+ Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
+ https://bugs.webkit.org/show_bug.cgi?id=199864
+
+ Reviewed by Saam Barati.
+
+ Source/_javascript_Core:
+
+ 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
+
+ 2019-08-09 Yusuke Suzuki <[email protected]>
+
+ Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
+ https://bugs.webkit.org/show_bug.cgi?id=199864
+
+ Reviewed by Saam Barati.
+
+ * 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.
+
+2019-08-12 Alan Coon <[email protected]>
+
Cherry-pick r248491. rdar://problem/54130636
Don't allow cross-origin iframes to autofocus
Added: branches/safari-608-branch/LayoutTests/http/tests/security/cross-frame-access-object-put-optimization-expected.txt (0 => 248577)
--- branches/safari-608-branch/LayoutTests/http/tests/security/cross-frame-access-object-put-optimization-expected.txt (rev 0)
+++ branches/safari-608-branch/LayoutTests/http/tests/security/cross-frame-access-object-put-optimization-expected.txt 2019-08-12 23:42:48 UTC (rev 248577)
@@ -0,0 +1,10 @@
+This tests that you can't get cross-origin object during [[Put]] operation.
+
+PASS data is not "null"
+PASS data is not "Cocoa"
+PASS data is not "null"
+PASS data is not "Cocoa"
+PASS: successfullyParsed should be 'true' and is.
+
+TEST COMPLETE
+
Added: branches/safari-608-branch/LayoutTests/http/tests/security/cross-frame-access-object-put-optimization.html (0 => 248577)
--- branches/safari-608-branch/LayoutTests/http/tests/security/cross-frame-access-object-put-optimization.html (rev 0)
+++ branches/safari-608-branch/LayoutTests/http/tests/security/cross-frame-access-object-put-optimization.html 2019-08-12 23:42:48 UTC (rev 248577)
@@ -0,0 +1,54 @@
+<html>
+<head>
+ <script src=""
+ <script src=""
+ <script>
+ jsTestIsAsync = true;
+
+ data = ""
+
+ // Set up listener for message from iframe
+ addEventListener('message', function(event) {
+ if (event.data == "finishedLoad")
+ doTest();
+ }, false);
+
+ function turnLeakedException(object) {
+ try {
+ object.setter = {toString: function() { return {} } };
+ } catch (e) {
+ let crossOriginFunctionConstructor = e.constructor.constructor;
+ data = ""
+ var element = document.getElementById("password");
+ if (!element)
+ return null;
+ return element.textContent;`)();
+ }
+ shouldNotBeEqualToString(`data`, `null`);
+ shouldNotBeEqualToString(`data`, "Cocoa");
+ data = ""
+ }
+
+ doTest = function()
+ {
+ let targetWindow = document.getElementById("target").contentWindow;
+
+ // putInlineSlow
+ turnLeakedException({__proto__: targetWindow.location});
+
+ // putToPrimitive
+ num = 1337;
+ num.__proto__.__proto__ = targetWindow.location;
+ turnLeakedException(num);
+
+ finishJSTest();
+ }
+ </script>
+</head>
+<body>
+ <div>This tests that you can't get cross-origin object during [[Put]] operation.</div>
+ <iframe id="target" src=""
+ <pre id="console"></pre>
+ <script src=""
+</body>
+</html>
Added: branches/safari-608-branch/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html (0 => 248577)
--- branches/safari-608-branch/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html (rev 0)
+++ branches/safari-608-branch/LayoutTests/http/tests/security/resources/cross-frame-iframe-for-object-put-optimization-test.html 2019-08-12 23:42:48 UTC (rev 248577)
@@ -0,0 +1,9 @@
+<h1 id="password">Cocoa</h1>
+<script>
+_onload_ = function() {
+ location.__defineSetter__('setter', function(value) {
+ throw new Error();
+ });
+ parent.postMessage("finishedLoad", "*");
+}
+</script>
Modified: branches/safari-608-branch/Source/_javascript_Core/ChangeLog (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/ChangeLog 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/ChangeLog 2019-08-12 23:42:48 UTC (rev 248577)
@@ -1,5 +1,147 @@
2019-08-12 Alan Coon <[email protected]>
+ Cherry-pick r248494. rdar://problem/54171876
+
+ Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
+ https://bugs.webkit.org/show_bug.cgi?id=199864
+
+ Reviewed by Saam Barati.
+
+ Source/_javascript_Core:
+
+ 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
+
+ 2019-08-09 Yusuke Suzuki <[email protected]>
+
+ Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
+ https://bugs.webkit.org/show_bug.cgi?id=199864
+
+ Reviewed by Saam Barati.
+
+ 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):
+
+2019-08-12 Alan Coon <[email protected]>
+
Cherry-pick r248027. rdar://problem/53836556
[JSC] Emit write barrier after storing instead of before storing
Modified: branches/safari-608-branch/Source/_javascript_Core/runtime/ClassInfo.h (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/runtime/ClassInfo.h 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/runtime/ClassInfo.h 2019-08-12 23:42:48 UTC (rev 248577)
@@ -79,6 +79,9 @@
using GetOwnPropertySlotByIndexFunctionPtr = bool (*)(JSObject*, ExecState*, unsigned, PropertySlot&);
GetOwnPropertySlotByIndexFunctionPtr WTF_METHOD_TABLE_ENTRY(getOwnPropertySlotByIndex);
+ using DoPutPropertySecurityCheckFunctionPtr = void (*)(JSObject*, ExecState*, PropertyName, PutPropertySlot&);
+ DoPutPropertySecurityCheckFunctionPtr METHOD_TABLE_ENTRY(doPutPropertySecurityCheck);
+
using ToThisFunctionPtr = JSValue (*)(JSCell*, ExecState*, ECMAMode);
ToThisFunctionPtr WTF_METHOD_TABLE_ENTRY(toThis);
@@ -167,6 +170,7 @@
&ClassName::deletePropertyByIndex, \
&ClassName::getOwnPropertySlot, \
&ClassName::getOwnPropertySlotByIndex, \
+ &ClassName::doPutPropertySecurityCheck, \
&ClassName::toThis, \
&ClassName::defaultValue, \
&ClassName::getOwnPropertyNames, \
Modified: branches/safari-608-branch/Source/_javascript_Core/runtime/JSCJSValue.cpp (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/runtime/JSCJSValue.cpp 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/runtime/JSCJSValue.cpp 2019-08-12 23:42:48 UTC (rev 248577)
@@ -162,18 +162,27 @@
return false;
JSValue prototype;
if (propertyName != vm.propertyNames->underscoreProto) {
- for (; !obj->structure(vm)->hasReadOnlyOrGetterSetterPropertiesExcludingProto(); obj = asObject(prototype)) {
+ while (true) {
+ Structure* structure = obj->structure(vm);
+ if (structure->hasReadOnlyOrGetterSetterPropertiesExcludingProto() || structure->typeInfo().hasPutPropertySecurityCheck())
+ break;
prototype = obj->getPrototype(vm, exec);
RETURN_IF_EXCEPTION(scope, false);
if (prototype.isNull())
return typeError(exec, scope, slot.isStrictMode(), ReadonlyPropertyWriteError);
+ obj = asObject(prototype);
}
}
for (; ; obj = asObject(prototype)) {
+ Structure* structure = obj->structure(vm);
+ if (UNLIKELY(structure->typeInfo().hasPutPropertySecurityCheck())) {
+ obj->methodTable(vm)->doPutPropertySecurityCheck(obj, exec, propertyName, slot);
+ RETURN_IF_EXCEPTION(scope, false);
+ }
unsigned attributes;
- PropertyOffset offset = obj->structure(vm)->get(vm, propertyName, attributes);
+ PropertyOffset offset = structure->get(vm, propertyName, attributes);
if (offset != invalidOffset) {
if (attributes & PropertyAttribute::ReadOnly)
return typeError(exec, scope, slot.isStrictMode(), ReadonlyPropertyWriteError);
Modified: branches/safari-608-branch/Source/_javascript_Core/runtime/JSCell.cpp (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/runtime/JSCell.cpp 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/runtime/JSCell.cpp 2019-08-12 23:42:48 UTC (rev 248577)
@@ -212,6 +212,11 @@
return false;
}
+void JSCell::doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&)
+{
+ RELEASE_ASSERT_NOT_REACHED();
+}
+
void JSCell::getOwnPropertyNames(JSObject*, ExecState*, PropertyNameArray&, EnumerationMode)
{
RELEASE_ASSERT_NOT_REACHED();
Modified: branches/safari-608-branch/Source/_javascript_Core/runtime/JSCell.h (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/runtime/JSCell.h 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/runtime/JSCell.h 2019-08-12 23:42:48 UTC (rev 248577)
@@ -265,6 +265,7 @@
static bool defineOwnProperty(JSObject*, ExecState*, PropertyName, const PropertyDescriptor&, bool shouldThrow);
static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
static bool getOwnPropertySlotByIndex(JSObject*, ExecState*, unsigned propertyName, PropertySlot&);
+ static NO_RETURN_DUE_TO_CRASH void doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&);
private:
friend class LLIntOffsetsExtractor;
Modified: branches/safari-608-branch/Source/_javascript_Core/runtime/JSObject.cpp (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/runtime/JSObject.cpp 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/runtime/JSObject.cpp 2019-08-12 23:42:48 UTC (rev 248577)
@@ -790,8 +790,13 @@
JSObject* obj = this;
for (;;) {
+ Structure* structure = obj->structure(vm);
+ if (UNLIKELY(structure->typeInfo().hasPutPropertySecurityCheck())) {
+ obj->methodTable(vm)->doPutPropertySecurityCheck(obj, exec, propertyName, slot);
+ RETURN_IF_EXCEPTION(scope, false);
+ }
unsigned attributes;
- PropertyOffset offset = obj->structure(vm)->get(vm, propertyName, attributes);
+ PropertyOffset offset = structure->get(vm, propertyName, attributes);
if (isValidOffset(offset)) {
if (attributes & PropertyAttribute::ReadOnly) {
ASSERT(this->prototypeChainMayInterceptStoreTo(vm, propertyName) || obj == this);
@@ -801,7 +806,7 @@
JSValue gs = obj->getDirect(offset);
if (gs.isGetterSetter()) {
// We need to make sure that we decide to cache this property before we potentially execute aribitrary JS.
- if (!structure(vm)->isDictionary())
+ if (!this->structure(vm)->isDictionary())
slot.setCacheableSetter(obj, offset);
bool result = callSetter(exec, slot.thisValue(), gs, value, slot.isStrictMode() ? StrictMode : NotStrictMode);
@@ -3468,6 +3473,11 @@
if (!result)
return false;
+
+ // FIXME: https://bugs.webkit.org/show_bug.cgi?id=200560
+ // This breaks the assumption that getOwnPropertySlot should return "own" property.
+ // We should fix DebuggerScope, ProxyObject etc. to remove this.
+ //
// DebuggerScope::getOwnPropertySlot() (and possibly others) may return attributes from the prototype chain
// but getOwnPropertyDescriptor() should only work for 'own' properties so we exit early if we detect that
// the property is not an own property.
Modified: branches/safari-608-branch/Source/_javascript_Core/runtime/JSObject.h (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/runtime/JSObject.h 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/runtime/JSObject.h 2019-08-12 23:42:48 UTC (rev 248577)
@@ -168,6 +168,7 @@
static bool getOwnPropertySlot(JSObject*, ExecState*, PropertyName, PropertySlot&);
JS_EXPORT_PRIVATE static bool getOwnPropertySlotByIndex(JSObject*, ExecState*, unsigned propertyName, PropertySlot&);
bool getOwnPropertySlotInline(ExecState*, PropertyName, PropertySlot&);
+ static void doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&);
// The key difference between this and getOwnPropertySlot is that getOwnPropertySlot
// currently returns incorrect results for the DOM window (with non-own properties)
@@ -1404,6 +1405,10 @@
return false;
}
+ALWAYS_INLINE void JSObject::doPutPropertySecurityCheck(JSObject*, ExecState*, PropertyName, PutPropertySlot&)
+{
+}
+
// It may seem crazy to inline a function this large but it makes a big difference
// since this is function very hot in variable lookup
template<bool checkNullStructure>
Modified: branches/safari-608-branch/Source/_javascript_Core/runtime/JSTypeInfo.h (248576 => 248577)
--- branches/safari-608-branch/Source/_javascript_Core/runtime/JSTypeInfo.h 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/_javascript_Core/runtime/JSTypeInfo.h 2019-08-12 23:42:48 UTC (rev 248577)
@@ -56,6 +56,7 @@
static const unsigned GetOwnPropertySlotIsImpureForPropertyAbsence = 1 << 14;
static const unsigned InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero = 1 << 15;
static const unsigned StructureIsImmortal = 1 << 16;
+static const unsigned HasPutPropertySecurityCheck = 1 << 17;
class TypeInfo {
public:
@@ -96,6 +97,7 @@
bool prohibitsPropertyCaching() const { return isSetOnFlags2(ProhibitsPropertyCaching); }
bool getOwnPropertySlotIsImpure() const { return isSetOnFlags2(GetOwnPropertySlotIsImpure); }
bool getOwnPropertySlotIsImpureForPropertyAbsence() const { return isSetOnFlags2(GetOwnPropertySlotIsImpureForPropertyAbsence); }
+ bool hasPutPropertySecurityCheck() const { return isSetOnFlags2(HasPutPropertySecurityCheck); }
bool newImpurePropertyFiresWatchpoints() const { return isSetOnFlags2(NewImpurePropertyFiresWatchpoints); }
bool isImmutablePrototypeExoticObject() const { return isSetOnFlags2(IsImmutablePrototypeExoticObject); }
bool interceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero() const { return isSetOnFlags2(InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero); }
Modified: branches/safari-608-branch/Source/WebCore/ChangeLog (248576 => 248577)
--- branches/safari-608-branch/Source/WebCore/ChangeLog 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/WebCore/ChangeLog 2019-08-12 23:42:48 UTC (rev 248577)
@@ -1,5 +1,105 @@
2019-08-12 Alan Coon <[email protected]>
+ Cherry-pick r248494. rdar://problem/54171876
+
+ Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
+ https://bugs.webkit.org/show_bug.cgi?id=199864
+
+ Reviewed by Saam Barati.
+
+ Source/_javascript_Core:
+
+ 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
+
+ 2019-08-09 Yusuke Suzuki <[email protected]>
+
+ Universal XSS in JSObject::putInlineSlow and JSValue::putToPrimitive
+ https://bugs.webkit.org/show_bug.cgi?id=199864
+
+ Reviewed by Saam Barati.
+
+ 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:
+
+2019-08-12 Alan Coon <[email protected]>
+
Cherry-pick r248491. rdar://problem/54130636
Don't allow cross-origin iframes to autofocus
Modified: branches/safari-608-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (248576 => 248577)
--- branches/safari-608-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp 2019-08-12 23:42:48 UTC (rev 248577)
@@ -264,6 +264,25 @@
return Base::getOwnPropertySlotByIndex(thisObject, state, index, slot);
}
+void JSDOMWindow::doPutPropertySecurityCheck(JSObject* cell, ExecState* state, PropertyName propertyName, PutPropertySlot&)
+{
+ VM& vm = state->vm();
+ auto scope = DECLARE_THROW_SCOPE(vm);
+
+ auto* thisObject = jsCast<JSDOMWindow*>(cell);
+ if (!thisObject->wrapped().frame())
+ return;
+
+ String errorMessage;
+ if (!BindingSecurity::shouldAllowAccessToDOMWindow(*state, thisObject->wrapped(), errorMessage)) {
+ // We only allow setting "location" attribute cross-origin.
+ if (propertyName == static_cast<JSVMClientData*>(vm.clientData)->builtinNames().locationPublicName())
+ return;
+ throwSecurityError(*state, scope, errorMessage);
+ return;
+ }
+}
+
bool JSDOMWindow::put(JSCell* cell, ExecState* state, PropertyName propertyName, JSValue value, PutPropertySlot& slot)
{
VM& vm = state->vm();
Modified: branches/safari-608-branch/Source/WebCore/bindings/js/JSLocationCustom.cpp (248576 => 248577)
--- branches/safari-608-branch/Source/WebCore/bindings/js/JSLocationCustom.cpp 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/WebCore/bindings/js/JSLocationCustom.cpp 2019-08-12 23:42:48 UTC (rev 248577)
@@ -126,6 +126,23 @@
return false;
}
+void JSLocation::doPutPropertySecurityCheck(JSObject* object, ExecState* state, PropertyName propertyName, PutPropertySlot&)
+{
+ auto* thisObject = jsCast<JSLocation*>(object);
+ ASSERT_GC_OBJECT_INHERITS(thisObject, info());
+
+ VM& vm = state->vm();
+
+ // Always allow assigning to the whole location.
+ // However, alllowing assigning of pieces might inadvertently disclose parts of the original location.
+ // So fall through to the access check for those.
+ if (propertyName == static_cast<JSVMClientData*>(vm.clientData)->builtinNames().hrefPublicName())
+ return;
+
+ // Block access and throw if there is a security error.
+ BindingSecurity::shouldAllowAccessToDOMWindow(state, thisObject->wrapped().window(), ThrowSecurityError);
+}
+
bool JSLocation::put(JSCell* cell, ExecState* state, PropertyName propertyName, JSValue value, PutPropertySlot& putPropertySlot)
{
auto* thisObject = jsCast<JSLocation*>(cell);
Modified: branches/safari-608-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (248576 => 248577)
--- branches/safari-608-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2019-08-12 23:42:48 UTC (rev 248577)
@@ -2639,6 +2639,11 @@
push(@headerContent, " static bool getOwnPropertySlotByIndex(JSC::JSObject*, JSC::ExecState*, unsigned propertyName, JSC::PropertySlot&);\n");
$structureFlags{"JSC::InterceptsGetOwnPropertySlotByIndexEvenWhenLengthIsNotZero"} = 1;
}
+
+ if ($interface->extendedAttributes->{CheckSecurity}) {
+ push(@headerContent, " static void doPutPropertySecurityCheck(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PutPropertySlot&);\n");
+ $structureFlags{"JSC::HasPutPropertySecurityCheck"} = 1;
+ }
if (InstanceOverridesGetOwnPropertyNames($interface)) {
push(@headerContent, " static void getOwnPropertyNames(JSC::JSObject*, JSC::ExecState*, JSC::PropertyNameArray&, JSC::EnumerationMode = JSC::EnumerationMode());\n");
Modified: branches/safari-608-branch/Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h (248576 => 248577)
--- branches/safari-608-branch/Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h 2019-08-12 23:42:43 UTC (rev 248576)
+++ branches/safari-608-branch/Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h 2019-08-12 23:42:48 UTC (rev 248577)
@@ -39,6 +39,7 @@
static JSC::JSObject* createPrototype(JSC::VM&, JSDOMGlobalObject&);
static JSC::JSObject* prototype(JSC::VM&, JSDOMGlobalObject&);
static TestActiveDOMObject* toWrapped(JSC::VM&, JSC::JSValue);
+ static void doPutPropertySecurityCheck(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PutPropertySlot&);
static void destroy(JSC::JSCell*);
DECLARE_INFO;
@@ -51,7 +52,7 @@
static JSC::JSValue getConstructor(JSC::VM&, const JSC::JSGlobalObject*);
static void heapSnapshot(JSCell*, JSC::HeapSnapshotBuilder&);
public:
- static const unsigned StructureFlags = Base::StructureFlags | JSC::HasStaticPropertyTable;
+ static const unsigned StructureFlags = Base::StructureFlags | JSC::HasPutPropertySecurityCheck | JSC::HasStaticPropertyTable;
protected:
JSTestActiveDOMObject(JSC::Structure*, JSDOMGlobalObject&, Ref<TestActiveDOMObject>&&);
