Title: [207341] trunk
Revision
207341
Author
mark....@apple.com
Date
2016-10-14 08:58:16 -0700 (Fri, 14 Oct 2016)

Log Message

JSON.parse should not modify frozen objects.
https://bugs.webkit.org/show_bug.cgi?id=163430

Reviewed by Saam Barati.

JSTests:

* stress/json-parse-on-frozen-object.js: Added.

Source/_javascript_Core:

The ES6 spec for JSON.parse (https://tc39.github.io/ecma262/#sec-json.parse and
https://tc39.github.io/ecma262/#sec-internalizejsonproperty) states that it uses
CreateDataProperty() (https://tc39.github.io/ecma262/#sec-createdataproperty) to
set values returned by a reviver.  The spec for CreateDataPropertyOrThrow states:

"This abstract operation creates a property whose attributes are set to the same
defaults used for properties created by the ECMAScript language assignment
operator. Normally, the property will not already exist. If it does exist and is
not configurable or if O is not extensible, [[DefineOwnProperty]] will return
false."

Note: CreateDataProperty() will not throw a TypeError.

Since the properties of frozen objects are not extensible, not configurable, and
not writeable, JSON.parse should fail to write to any frozen objects.  Similarly,
JSON.parse should fail to delete properties in frozen objects.

In JSON.parse(), we previously write to array elements using the form of
putDirectIndex() that uses mode PutDirectIndexLikePutDirect.  This makes it so
that the write (i.e. put) is always successful.  We've now fixed this to use
PutDirectIndexShouldNotThrow mode instead, which will fail to put the element if
the array is not writeable.

Also changed Walker::walk() to use the version of methodTable() that takes a VM&
since the VM& is already available.

* runtime/JSONObject.cpp:
(JSC::Walker::walk):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (207340 => 207341)


--- trunk/JSTests/ChangeLog	2016-10-14 15:39:59 UTC (rev 207340)
+++ trunk/JSTests/ChangeLog	2016-10-14 15:58:16 UTC (rev 207341)
@@ -1,3 +1,12 @@
+2016-10-14  Mark Lam  <mark....@apple.com>
+
+        JSON.parse should not modify frozen objects.
+        https://bugs.webkit.org/show_bug.cgi?id=163430
+
+        Reviewed by Saam Barati.
+
+        * stress/json-parse-on-frozen-object.js: Added.
+
 2016-10-14  Joseph Pecoraro  <pecor...@apple.com>
 
         test262: Failure with RegExp.prototype.compile when pattern is undefined

Added: trunk/JSTests/stress/json-parse-on-frozen-object.js (0 => 207341)


--- trunk/JSTests/stress/json-parse-on-frozen-object.js	                        (rev 0)
+++ trunk/JSTests/stress/json-parse-on-frozen-object.js	2016-10-14 15:58:16 UTC (rev 207341)
@@ -0,0 +1,75 @@
+//@ runFTLNoCJIT
+
+function shouldEqual(testId, actual, expected) {
+    if (actual != expected) {
+        throw testId + ": ERROR: expect " + expected + ", actual " + actual;
+    }
+}
+
+function frozenArrayReviver(k, v) {
+    if (k === "a") {
+        this.b = Object.freeze(["unmodifiable"]);
+        return v;
+    }
+    if (k === "0")
+        return "modified";
+    return v;
+}
+
+function frozenArrayLikeObjectReviver(k, v) {
+    if (k === "a") {
+        var obj = {};
+        obj[0] = 'unmodifiable';
+        obj.length = 1; 
+        this.b = Object.freeze(obj);
+        return v;
+    }
+    if (k === "0")
+        return "modified";
+    return v;
+}
+
+function frozenArrayReviverWithDelete(k, v) {
+    if (k === "a") {
+        this.b = Object.freeze(["unmodifiable"]);
+        return v;
+    }
+    if (k === "0")
+        return undefined;
+    return v;
+}
+
+function frozenArrayLikeObjectReviverWithDelete(k, v) {
+    if (k === "a") {
+        var obj = {};
+        obj[0] = 'unmodifiable';
+        obj.length = 1; 
+        this.b = Object.freeze(obj);
+        return v;
+    }
+    if (k === "0")
+        return undefined;
+    return v;
+}
+
+function runTest(testId, reviver, expectedValue, expectedException) {
+    let numIterations = 10000;
+    for (var i = 0; i < numIterations; i++) {
+        var exception = undefined;
+
+        var obj;
+        try {
+            obj = JSON.parse('{ "a": 0, "b": 0 }', reviver);
+        } catch (e) {
+            exception = "" + e;
+            exception = exception.substr(0, 10); // Search for "TypeError:".
+        }
+        shouldEqual(testId, exception, expectedException);
+        shouldEqual(testId, obj.b[0], expectedValue);
+    }
+}
+
+runTest(10000, frozenArrayReviver,                     "unmodifiable", undefined);
+runTest(10001, frozenArrayLikeObjectReviver,           "unmodifiable", undefined);
+runTest(10002, frozenArrayReviverWithDelete,           "unmodifiable", undefined);
+runTest(10003, frozenArrayLikeObjectReviverWithDelete, "unmodifiable", undefined);

Modified: trunk/Source/_javascript_Core/ChangeLog (207340 => 207341)


--- trunk/Source/_javascript_Core/ChangeLog	2016-10-14 15:39:59 UTC (rev 207340)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-10-14 15:58:16 UTC (rev 207341)
@@ -1,3 +1,39 @@
+2016-10-14  Mark Lam  <mark....@apple.com>
+
+        JSON.parse should not modify frozen objects.
+        https://bugs.webkit.org/show_bug.cgi?id=163430
+
+        Reviewed by Saam Barati.
+
+        The ES6 spec for JSON.parse (https://tc39.github.io/ecma262/#sec-json.parse and
+        https://tc39.github.io/ecma262/#sec-internalizejsonproperty) states that it uses
+        CreateDataProperty() (https://tc39.github.io/ecma262/#sec-createdataproperty) to
+        set values returned by a reviver.  The spec for CreateDataPropertyOrThrow states:
+
+        "This abstract operation creates a property whose attributes are set to the same
+        defaults used for properties created by the ECMAScript language assignment
+        operator. Normally, the property will not already exist. If it does exist and is
+        not configurable or if O is not extensible, [[DefineOwnProperty]] will return
+        false."
+
+        Note: CreateDataProperty() will not throw a TypeError.
+
+        Since the properties of frozen objects are not extensible, not configurable, and
+        not writeable, JSON.parse should fail to write to any frozen objects.  Similarly,
+        JSON.parse should fail to delete properties in frozen objects.
+
+        In JSON.parse(), we previously write to array elements using the form of
+        putDirectIndex() that uses mode PutDirectIndexLikePutDirect.  This makes it so
+        that the write (i.e. put) is always successful.  We've now fixed this to use
+        PutDirectIndexShouldNotThrow mode instead, which will fail to put the element if
+        the array is not writeable.
+
+        Also changed Walker::walk() to use the version of methodTable() that takes a VM&
+        since the VM& is already available.
+
+        * runtime/JSONObject.cpp:
+        (JSC::Walker::walk):
+
 2016-10-14  Joseph Pecoraro  <pecor...@apple.com>
 
         test262: Failure with RegExp.prototype.compile when pattern is undefined

Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (207340 => 207341)


--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp	2016-10-14 15:39:59 UTC (rev 207340)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp	2016-10-14 15:58:16 UTC (rev 207341)
@@ -631,7 +631,7 @@
                     inValue = array->getIndexQuickly(index);
                 else {
                     PropertySlot slot(array, PropertySlot::InternalMethodType::Get);
-                    if (array->methodTable()->getOwnPropertySlotByIndex(array, m_exec, index, slot))
+                    if (array->methodTable(vm)->getOwnPropertySlotByIndex(array, m_exec, index, slot))
                         inValue = slot.getValue(m_exec, index);
                     else
                         inValue = jsUndefined();
@@ -649,9 +649,9 @@
                 JSArray* array = arrayStack.peek();
                 JSValue filteredValue = callReviver(array, jsString(m_exec, String::number(indexStack.last())), outValue);
                 if (filteredValue.isUndefined())
-                    array->methodTable()->deletePropertyByIndex(array, m_exec, indexStack.last());
+                    array->methodTable(vm)->deletePropertyByIndex(array, m_exec, indexStack.last());
                 else
-                    array->putDirectIndex(m_exec, indexStack.last(), filteredValue);
+                    array->putDirectIndex(m_exec, indexStack.last(), filteredValue, 0, PutDirectIndexShouldNotThrow);
                 RETURN_IF_EXCEPTION(scope, JSValue());
                 indexStack.last()++;
                 goto arrayStartVisitMember;
@@ -667,7 +667,7 @@
                 objectStack.push(object);
                 indexStack.append(0);
                 propertyStack.append(PropertyNameArray(m_exec, PropertyNameMode::Strings));
-                object->methodTable()->getOwnPropertyNames(object, m_exec, propertyStack.last(), EnumerationMode());
+                object->methodTable(vm)->getOwnPropertyNames(object, m_exec, propertyStack.last(), EnumerationMode());
                 RETURN_IF_EXCEPTION(scope, JSValue());
             }
             objectStartVisitMember:
@@ -684,7 +684,7 @@
                     break;
                 }
                 PropertySlot slot(object, PropertySlot::InternalMethodType::Get);
-                if (object->methodTable()->getOwnPropertySlot(object, m_exec, properties[index], slot))
+                if (object->methodTable(vm)->getOwnPropertySlot(object, m_exec, properties[index], slot))
                     inValue = slot.getValue(m_exec, properties[index]);
                 else
                     inValue = jsUndefined();
@@ -705,9 +705,9 @@
                 PutPropertySlot slot(object);
                 JSValue filteredValue = callReviver(object, jsString(m_exec, prop.string()), outValue);
                 if (filteredValue.isUndefined())
-                    object->methodTable()->deleteProperty(object, m_exec, prop);
+                    object->methodTable(vm)->deleteProperty(object, m_exec, prop);
                 else
-                    object->methodTable()->put(object, m_exec, prop, filteredValue, slot);
+                    object->methodTable(vm)->put(object, m_exec, prop, filteredValue, slot);
                 RETURN_IF_EXCEPTION(scope, JSValue());
                 indexStack.last()++;
                 goto objectStartVisitMember;
@@ -731,7 +731,7 @@
     }
     JSObject* finalHolder = constructEmptyObject(m_exec);
     PutPropertySlot slot(finalHolder);
-    finalHolder->methodTable()->put(finalHolder, m_exec, vm.propertyNames->emptyIdentifier, outValue, slot);
+    finalHolder->methodTable(vm)->put(finalHolder, m_exec, vm.propertyNames->emptyIdentifier, outValue, slot);
     return callReviver(finalHolder, jsEmptyString(m_exec), outValue);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to