Title: [207785] trunk/Source/_javascript_Core
Revision
207785
Author
achristen...@apple.com
Date
2016-10-24 15:51:59 -0700 (Mon, 24 Oct 2016)

Log Message

JSONParse should not crash with null Strings
https://bugs.webkit.org/show_bug.cgi?id=163918
<rdar://problem/28834095>

Reviewed by Michael Saboff.

When JSONParse is called with a null String, it calls String::is8bit, which dereferences a null pointer.
This is happening with new work in the Fetch API, but callers of JSONParse should not have to check
if the String is null.

* API/tests/JSONParseTest.cpp: Added.
(testJSONParse):
* API/tests/JSONParseTest.h: Added.
* API/tests/testapi.c:
(main):
Test parsing null Strings.  They should have the same result as parsing empty Strings.
* _javascript_Core.xcodeproj/project.pbxproj:
* runtime/JSONObject.cpp:
(JSC::JSONParse):
Check for null Strings.
* shell/PlatformWin.cmake:

Modified Paths

Added Paths

Diff

Added: trunk/Source/_javascript_Core/API/tests/JSONParseTest.cpp (0 => 207785)


--- trunk/Source/_javascript_Core/API/tests/JSONParseTest.cpp	                        (rev 0)
+++ trunk/Source/_javascript_Core/API/tests/JSONParseTest.cpp	2016-10-24 22:51:59 UTC (rev 207785)
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "JSONParseTest.h"
+
+#include "JSCInlines.h"
+#include "JSGlobalObject.h"
+#include "JSONObject.h"
+#include "VM.h"
+#include <wtf/RefPtr.h>
+
+using namespace JSC;
+
+int testJSONParse()
+{
+    bool failed = false;
+
+    RefPtr<VM> vm = VM::create();
+
+    JSLockHolder locker(vm.get());
+    JSGlobalObject* globalObject = JSGlobalObject::create(*vm, JSGlobalObject::createStructure(*vm, jsNull()));
+
+    ExecState* exec = globalObject->globalExec();
+    JSValue v0 = JSONParse(exec, "");
+    JSValue v1 = JSONParse(exec, "#$%^");
+    JSValue v2 = JSONParse(exec, String());
+    UChar emptyUCharArray[1] = { '\0' };
+    JSValue v3 = JSONParse(exec, String(emptyUCharArray, 0));
+    JSValue v4;
+    JSValue v5 = JSONParse(exec, "123");
+
+    failed = failed || (v0 != v1);
+    failed = failed || (v1 != v2);
+    failed = failed || (v2 != v3);
+    failed = failed || (v3 != v4);
+    failed = failed || (v4 == v5);
+
+    vm = nullptr;
+
+    if (failed)
+        printf("FAIL: JSONParse String test.\n");
+    else
+        printf("PASS: JSONParse String test.\n");
+
+    return failed;
+}

Added: trunk/Source/_javascript_Core/API/tests/JSONParseTest.h (0 => 207785)


--- trunk/Source/_javascript_Core/API/tests/JSONParseTest.h	                        (rev 0)
+++ trunk/Source/_javascript_Core/API/tests/JSONParseTest.h	2016-10-24 22:51:59 UTC (rev 207785)
@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2016 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+    
+int testJSONParse();
+    
+#ifdef __cplusplus
+} /* extern "C" */
+#endif

Modified: trunk/Source/_javascript_Core/API/tests/testapi.c (207784 => 207785)


--- trunk/Source/_javascript_Core/API/tests/testapi.c	2016-10-24 22:09:29 UTC (rev 207784)
+++ trunk/Source/_javascript_Core/API/tests/testapi.c	2016-10-24 22:51:59 UTC (rev 207785)
@@ -44,6 +44,7 @@
 #include "ExecutionTimeLimitTest.h"
 #include "FunctionOverridesTest.h"
 #include "GlobalContextWithFinalizerTest.h"
+#include "JSONParseTest.h"
 #include "PingPongStackOverflowTest.h"
 #include "TypedArrayCTest.h"
 
@@ -1129,8 +1130,6 @@
     testObjectiveCAPI();
 #endif
 
-
-
     const char *scriptPath = "testapi.js";
     if (argc > 1) {
         scriptPath = argv[1];
@@ -1889,6 +1888,7 @@
     failed = testFunctionOverrides() || failed;
     failed = testGlobalContextWithFinalizer() || failed;
     failed = testPingPongStackOverflow() || failed;
+    failed = testJSONParse() || failed;
 
     // Clear out local variables pointing at JSObjectRefs to allow their values to be collected
     function = NULL;

Modified: trunk/Source/_javascript_Core/ChangeLog (207784 => 207785)


--- trunk/Source/_javascript_Core/ChangeLog	2016-10-24 22:09:29 UTC (rev 207784)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-10-24 22:51:59 UTC (rev 207785)
@@ -1,3 +1,27 @@
+2016-10-24  Alex Christensen  <achristen...@webkit.org>
+
+        JSONParse should not crash with null Strings
+        https://bugs.webkit.org/show_bug.cgi?id=163918
+        <rdar://problem/28834095>
+
+        Reviewed by Michael Saboff.
+
+        When JSONParse is called with a null String, it calls String::is8bit, which dereferences a null pointer.
+        This is happening with new work in the Fetch API, but callers of JSONParse should not have to check
+        if the String is null.
+
+        * API/tests/JSONParseTest.cpp: Added.
+        (testJSONParse):
+        * API/tests/JSONParseTest.h: Added.
+        * API/tests/testapi.c:
+        (main):
+        Test parsing null Strings.  They should have the same result as parsing empty Strings.
+        * _javascript_Core.xcodeproj/project.pbxproj:
+        * runtime/JSONObject.cpp:
+        (JSC::JSONParse):
+        Check for null Strings.
+        * shell/PlatformWin.cmake:
+
 2016-10-24  Devin Rousso  <dcrousso+web...@gmail.com>
 
         Web Inspector: Scope chain shouldn't show empty Closure sections

Modified: trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj (207784 => 207785)


--- trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj	2016-10-24 22:09:29 UTC (rev 207784)
+++ trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj	2016-10-24 22:51:59 UTC (rev 207785)
@@ -1243,6 +1243,7 @@
 		53FA2AE31CF380390022711D /* LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 53FA2AE21CF380390022711D /* LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp */; };
 		53FD04D31D7AB277003287D3 /* WasmCallingConvention.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 53FD04D11D7AB187003287D3 /* WasmCallingConvention.cpp */; };
 		53FD04D41D7AB291003287D3 /* WasmCallingConvention.h in Headers */ = {isa = PBXBuildFile; fileRef = 53FD04D21D7AB187003287D3 /* WasmCallingConvention.h */; };
+		5C4E8E961DBEBE620036F1FC /* JSONParseTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5C4E8E941DBEBDA20036F1FC /* JSONParseTest.cpp */; };
 		5D5D8AD10E0D0EBE00F9C692 /* libedit.dylib in Frameworks */ = {isa = PBXBuildFile; fileRef = 5D5D8AD00E0D0EBE00F9C692 /* libedit.dylib */; };
 		5DBB151B131D0B310056AD36 /* testapi.js in Copy Support Script */ = {isa = PBXBuildFile; fileRef = 14D857740A4696C80032146C /* testapi.js */; };
 		5DBB1525131D0BD70056AD36 /* minidom.js in Copy Support Script */ = {isa = PBXBuildFile; fileRef = 1412110D0A48788700480255 /* minidom.js */; };
@@ -3548,6 +3549,8 @@
 		53FA2AE21CF380390022711D /* LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = LLIntPrototypeLoadAdaptiveStructureWatchpoint.cpp; sourceTree = "<group>"; };
 		53FD04D11D7AB187003287D3 /* WasmCallingConvention.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WasmCallingConvention.cpp; sourceTree = "<group>"; };
 		53FD04D21D7AB187003287D3 /* WasmCallingConvention.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WasmCallingConvention.h; sourceTree = "<group>"; };
+		5C4E8E941DBEBDA20036F1FC /* JSONParseTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = JSONParseTest.cpp; path = API/tests/JSONParseTest.cpp; sourceTree = "<group>"; };
+		5C4E8E951DBEBDA20036F1FC /* JSONParseTest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = JSONParseTest.h; path = API/tests/JSONParseTest.h; sourceTree = "<group>"; };
 		5D5D8AD00E0D0EBE00F9C692 /* libedit.dylib */ = {isa = PBXFileReference; lastKnownFileType = "compiled.mach-o.dylib"; name = libedit.dylib; path = /usr/lib/libedit.dylib; sourceTree = "<absolute>"; };
 		5DAFD6CB146B686300FBEFB4 /* JSC.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = JSC.xcconfig; sourceTree = "<group>"; };
 		5DDDF44614FEE72200B4FB4D /* LLIntDesiredOffsets.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = LLIntDesiredOffsets.h; path = LLIntOffsets/LLIntDesiredOffsets.h; sourceTree = BUILT_PRODUCTS_DIR; };
@@ -5283,6 +5286,8 @@
 				FE0D4A081ABA2437002F54BF /* GlobalContextWithFinalizerTest.h */,
 				C2181FC018A948FB0025A235 /* JSExportTests.h */,
 				C2181FC118A948FB0025A235 /* JSExportTests.mm */,
+				5C4E8E941DBEBDA20036F1FC /* JSONParseTest.cpp */,
+				5C4E8E951DBEBDA20036F1FC /* JSONParseTest.h */,
 				144005170A531CB50005F061 /* minidom */,
 				FEDA50D41B97F442009A3B4F /* PingPongStackOverflowTest.cpp */,
 				FEDA50D51B97F4D9009A3B4F /* PingPongStackOverflowTest.h */,
@@ -9195,6 +9200,7 @@
 				C29ECB031804D0ED00D2CBB4 /* CurrentThisInsideBlockGetterTest.mm in Sources */,
 				C20328201981979D0088B499 /* CustomGlobalObjectClassTest.c in Sources */,
 				C288B2DE18A54D3E007BE40B /* DateTests.mm in Sources */,
+				5C4E8E961DBEBE620036F1FC /* JSONParseTest.cpp in Sources */,
 				FE0D4A061AB8DD0A002F54BF /* ExecutionTimeLimitTest.cpp in Sources */,
 				FE0D4A091ABA2437002F54BF /* GlobalContextWithFinalizerTest.cpp in Sources */,
 				C2181FC218A948FB0025A235 /* JSExportTests.mm in Sources */,

Modified: trunk/Source/_javascript_Core/runtime/JSONObject.cpp (207784 => 207785)


--- trunk/Source/_javascript_Core/runtime/JSONObject.cpp	2016-10-24 22:09:29 UTC (rev 207784)
+++ trunk/Source/_javascript_Core/runtime/JSONObject.cpp	2016-10-24 22:51:59 UTC (rev 207785)
@@ -791,6 +791,9 @@
 {
     LocalScope scope(exec->vm());
 
+    if (json.isNull())
+        return JSValue();
+
     if (json.is8Bit()) {
         LiteralParser<LChar> jsonParser(exec, json.characters8(), json.length(), StrictJSON);
         return jsonParser.tryLiteralParse();

Modified: trunk/Source/_javascript_Core/shell/PlatformWin.cmake (207784 => 207785)


--- trunk/Source/_javascript_Core/shell/PlatformWin.cmake	2016-10-24 22:09:29 UTC (rev 207784)
+++ trunk/Source/_javascript_Core/shell/PlatformWin.cmake	2016-10-24 22:51:59 UTC (rev 207785)
@@ -31,9 +31,10 @@
     ../API/tests/ExecutionTimeLimitTest.cpp
     ../API/tests/FunctionOverridesTest.cpp
     ../API/tests/GlobalContextWithFinalizerTest.cpp
+    ../API/tests/JSONParseTest.cpp
     ../API/tests/PingPongStackOverflowTest.cpp
     ../API/tests/testapi.c
-   ../API/tests/TypedArrayCTest.cpp
+    ../API/tests/TypedArrayCTest.cpp
 )
 set_source_files_properties(../API/tests/CustomGlobalObjectClassTest.c PROPERTIES COMPILE_FLAGS "/TP /MT")
 set_source_files_properties(../API/tests/testapi.c PROPERTIES COMPILE_FLAGS "/TP /MT")
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to