Changeset 207341 in webkit

Timestamp:

Oct 14, 2016 8:58:16 AM (8 years ago)

Author:

mark.lam@apple.com

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/JavaScriptCore:

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/json-parse-on-frozen-object.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSONObject.cpp (modified) (6 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2016-10-14  Mark Lam  <mark.lam@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  <pecoraro@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-10-14  Mark Lam  <mark.lam@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  <pecoraro@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSONObject.cpp

@@ -632 +632 @@
                 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
@@ -650 +650 @@
                 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()++;
@@ -668 +668 @@
                 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());
             }
@@ -685 +685 @@
                 }
                 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
@@ -706 +706 @@
                 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()++;
@@ -732 +732 @@
     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);
 }