Merge r245028 - Correct JSON parser to address unterminated escape ch…

…aracter https://bugs.webkit.org/show_bug.cgi?id=197582 <rdar://problem/50459177> Reviewed by Alex Christensen. Source/WTF: Correct JSON parser code to properly deal with unterminated escape characters. * wtf/JSONValues.cpp: (WTF::JSONImpl::decodeString): (WTF::JSONImpl::parseStringToken): LayoutTests: * applicationmanifest/display-mode-bad-manifest-expected.txt: * applicationmanifest/display-mode-bad-manifest.html: * applicationmanifest/resources/bad.manifest: Added. * js/resources/JSON-parse.js: Add test case for unterminated escape. * js/dom/JSON-parse-expected.txt: Add new test case. * TestWebKitAPI/Tests/WTF/JSONValue.cpp: Add new false test case for unterminated escape character.
WebKit · May 9, 2019 · 0b5df36 · 0b5df36
1 parent 4cfc4b8
commit 0b5df36
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 3 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,19 @@
+2019-05-07  Brent Fulgham  <bfulgham@apple.com>
+
+        Correct JSON parser to address unterminated escape character
+        https://bugs.webkit.org/show_bug.cgi?id=197582
+        <rdar://problem/50459177>
+
+        Reviewed by Alex Christensen.
+
+        * applicationmanifest/display-mode-bad-manifest-expected.txt:
+        * applicationmanifest/display-mode-bad-manifest.html:
+        * applicationmanifest/resources/bad.manifest: Added.
+        * js/resources/JSON-parse.js: Add test case for unterminated escape.
+        * js/dom/JSON-parse-expected.txt: Add new test case.
+        * TestWebKitAPI/Tests/WTF/JSONValue.cpp: Add new false test case
+          for unterminated escape character.
+
 2019-04-08  Yusuke Suzuki  <ysuzuki@apple.com>
 
         Unreviewed, rolling in r243948 with test fix

diff --git a/LayoutTests/applicationmanifest/display-mode-bad-manifest-expected.txt b/LayoutTests/applicationmanifest/display-mode-bad-manifest-expected.txt
@@ -0,0 +1 @@
+(display-mode) (display-mode: browser)
diff --git a/LayoutTests/applicationmanifest/display-mode-bad-manifest.html b/LayoutTests/applicationmanifest/display-mode-bad-manifest.html
@@ -0,0 +1,32 @@
+<!-- webkit-test-runner [ applicationManifest=resources/bad.manifest ] -->
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<style>
+div { display: none; }
+@media (display-mode) {
+    .display-mode-null { display: inline; }
+}
+@media (display-mode: browser) {
+    .display-mode-browser { display: inline; }
+}
+@media (display-mode: minimal-ui) {
+    .display-mode-minimal-ui { display: inline; }
+}
+@media (display-mode: standalone) {
+    .display-mode-standalone { display: inline; }
+}
+@media (display-mode: fullscreen) {
+    .display-mode-fullscreen { display: inline; }
+}
+@media (display-mode: invalid-value) {
+    .display-mode-invalid-value { display: inline; }
+}
+</style>
+<div class="display-mode-null">(display-mode)</div>
+<div class="display-mode-browser">(display-mode: browser)</div>
+<div class="display-mode-minimal-ui">(display-mode: minimal-ui)</div>
+<div class="display-mode-standalone">(display-mode: standalone)</div>
+<div class="display-mode-fullscreen">(display-mode: fullscreen)</div>
+<div class="display-mode-invalid-value">(display-mode: invalid-value)</div>
diff --git a/LayoutTests/applicationmanifest/resources/bad.manifest b/LayoutTests/applicationmanifest/resources/bad.manifest
@@ -0,0 +1 @@
+"\
diff --git a/LayoutTests/js/dom/JSON-parse-expected.txt b/LayoutTests/js/dom/JSON-parse-expected.txt
@@ -347,6 +347,10 @@ function (jsonObject){
         return jsonObject.parse('false');
     }
 PASS JSON.stringify(tests[i](nativeJSON)) is JSON.stringify(tests[i](JSON))
+function (jsonObject){
+        return jsonObject.parse('\\')
+    }
+PASS tests[i](nativeJSON) threw exception SyntaxError: JSON Parse error: Unrecognized token '\'.
 function (jsonObject){
         return jsonObject.parse(JSON.stringify(simpleObject));
     }

diff --git a/LayoutTests/js/resources/JSON-parse.js b/LayoutTests/js/resources/JSON-parse.js
@@ -308,6 +308,10 @@ function createTests() {
     result.push(function(jsonObject){
         return jsonObject.parse('false');
     });
+    result.push(function(jsonObject){
+        return jsonObject.parse('\\')
+    });
+    result[result.length - 1].throws = true;
     var simpleArray = ['a', 'b', 'c'];
     var simpleObject = {a:"1", b:"2", c:"3"};
     var complexArray = ['a', 'b', 'c',,,simpleObject, simpleArray, [simpleObject,simpleArray]];

diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
@@ -1,3 +1,18 @@
+2019-05-07  Brent Fulgham  <bfulgham@apple.com>
+
+        Correct JSON parser to address unterminated escape character
+        https://bugs.webkit.org/show_bug.cgi?id=197582
+        <rdar://problem/50459177>
+
+        Reviewed by Alex Christensen.
+
+        Correct JSON parser code to properly deal with unterminated escape
+        characters.
+
+        * wtf/JSONValues.cpp:
+        (WTF::JSONImpl::decodeString):
+        (WTF::JSONImpl::parseStringToken):
+
 2019-04-10  Enrique Ocaña González  <eocanha@igalia.com>
 
         [WPE] Avoid async IO starving timers

diff --git a/Source/WTF/wtf/JSONValues.cpp b/Source/WTF/wtf/JSONValues.cpp
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  * Copyright (C) 2014 University of Washington. All rights reserved.
- * Copyright (C) 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 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
@@ -163,7 +163,7 @@ bool parseStringToken(const UChar* start, const UChar* end, const UChar** tokenE
 {
     while (start < end) {
         UChar c = *start++;
-        if ('\\' == c) {
+        if ('\\' == c && start < end) {
             c = *start++;
             // Make sure the escaped char is valid.
             switch (c) {
@@ -269,6 +269,8 @@ bool decodeString(const UChar* start, const UChar* end, StringBuilder& output)
             output.append(c);
             continue;
         }
+        if (UNLIKELY(start >= end))
+            return false;
         c = *start++;
         switch (c) {
         case '"':
@@ -294,10 +296,14 @@ bool decodeString(const UChar* start, const UChar* end, StringBuilder& output)
             c = '\v';
             break;
         case 'x':
+            if (UNLIKELY(start + 1 >= end))
+                return false;
             c = toASCIIHexValue(start[0], start[1]);
             start += 2;
             break;
         case 'u':
+            if (UNLIKELY(start + 3 >= end))
+                return false;
             c = toASCIIHexValue(start[0], start[1]) << 8 | toASCIIHexValue(start[2], start[3]);
             start += 4;
             break;

diff --git a/Tools/TestWebKitAPI/Tests/WTF/JSONValue.cpp b/Tools/TestWebKitAPI/Tests/WTF/JSONValue.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -655,10 +655,25 @@ TEST(JSONValue, ParseJSON)
         EXPECT_TRUE(JSON::Value::parseJSON(" 1 ", value));
         EXPECT_TRUE(JSON::Value::parseJSON(" {} ", value));
         EXPECT_TRUE(JSON::Value::parseJSON(" [] ", value));
+        EXPECT_TRUE(JSON::Value::parseJSON("\"\\xFF\"", value));
+        EXPECT_TRUE(JSON::Value::parseJSON("\"\\u1234\"", value));
 
         EXPECT_FALSE(JSON::Value::parseJSON("1 1", value));
         EXPECT_FALSE(JSON::Value::parseJSON("{} {}", value));
         EXPECT_FALSE(JSON::Value::parseJSON("[] []", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\xF", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\xF\"", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\xF \"", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u1", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u1\"", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u1   \"", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u12", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u12\"", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u12  \"", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u123", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u123\"", value));
+        EXPECT_FALSE(JSON::Value::parseJSON("\"\\u123 \"", value));
     }
 }