From 5809e51ae405d763700ec19083009a2a1cdbfdbc Mon Sep 17 00:00:00 2001
From: Tyson Andre <tyson.andre@uwaterloo.ca>
Date: Fri, 30 Sep 2022 12:13:16 -0400
Subject: [PATCH] fix: Reject surrogate pairs with invalid low surrogate
 (#1896)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #1894

Reject low surrogates outside of the range U+DC00—U+DFFF

Related to https://unicodebook.readthedocs.io/unicode_encodings.html#utf-16-surrogate-pairs

A surrogate pair should consist of a high surrogate and low surrogate.
They're used to represent 0x010000-0x10FFFF in the JSON spec because
the JavaScript specification originally only supported `\uXXXX`.

Previously, simdjson would accept some combinations of valid high
surrogates and invalid low surrogates due to a bug in the check.
(e.g. `\uD888\u1234` was accepted)

U+D800—U+DBFF (1,024 code points): high surrogates
U+DC00—U+DFFF (1,024 code points): low surrogates
---
 src/generic/stage2/stringparsing.h     | 15 +++++-----
 tests/ondemand/ondemand_misc_tests.cpp | 39 ++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/src/generic/stage2/stringparsing.h b/src/generic/stage2/stringparsing.h
index 47ecd025e..e330b86bc 100644
--- a/src/generic/stage2/stringparsing.h
+++ b/src/generic/stage2/stringparsing.h
@@ -58,17 +58,18 @@ simdjson_inline bool handle_unicode_codepoint(const uint8_t **src_ptr,
     }
     uint32_t code_point_2 = jsoncharutils::hex_to_u32_nocheck(*src_ptr + 2);
 
-    // if the first code point is invalid we will get here, as we will go past
-    // the check for being outside the Basic Multilingual plane. If we don't
-    // find a \u immediately afterwards we fail out anyhow, but if we do,
-    // this check catches both the case of the first code point being invalid
-    // or the second code point being invalid.
-    if ((code_point | code_point_2) >> 16) {
+    // We have already checked that the high surrogate is valid and
+    // (code_point - 0xd800) < 1024.
+    //
+    // Check that code_point_2 is in the range 0xdc00..0xdfff
+    // and that code_point_2 was parsed from valid hex.
+    uint32_t low_bit = code_point_2 - 0xdc00;
+    if (low_bit >> 10) {
       return false;
     }
 
     code_point =
-        (((code_point - 0xd800) << 10) | (code_point_2 - 0xdc00)) + 0x10000;
+        (((code_point - 0xd800) << 10) | low_bit) + 0x10000;
     *src_ptr += 6;
   } else if (code_point >= 0xdc00 && code_point <= 0xdfff) {
       // If we encounter a low surrogate (not preceded by a high surrogate)
diff --git a/tests/ondemand/ondemand_misc_tests.cpp b/tests/ondemand/ondemand_misc_tests.cpp
index 7325e5b44..1a422f534 100644
--- a/tests/ondemand/ondemand_misc_tests.cpp
+++ b/tests/ondemand/ondemand_misc_tests.cpp
@@ -63,6 +63,42 @@ namespace misc_tests {
     TEST_SUCCEED();
   }
 
+  // Test a surrogate pair with the low surrogate out of range
+  bool issue1894() {
+    TEST_START();
+    ondemand::parser parser;
+    auto json = R"("\uD888\u1234")"_padded;
+    ondemand::document doc;
+    ASSERT_SUCCESS(parser.iterate(json).get(doc));
+    std::string_view view;
+    ASSERT_ERROR(doc.get_string().get(view), STRING_ERROR);
+    TEST_SUCCEED();
+  }
+
+  bool issue1894toolarge() {
+    TEST_START();
+    ondemand::parser parser;
+    auto json = R"("\uD888\uE000")"_padded;
+    ondemand::document doc;
+    ASSERT_SUCCESS(parser.iterate(json).get(doc));
+    std::string_view view;
+    ASSERT_ERROR(doc.get_string().get(view), STRING_ERROR);
+    TEST_SUCCEED();
+  }
+
+  // Test the smallest surrogate pair, largest surrogate pair, and a surrogate pair in range.
+  bool issue1894success() {
+    TEST_START();
+    ondemand::parser parser;
+    auto json = R"("\uD888\uDC00\uD800\uDC00\uDBFF\uDFFF")"_padded;
+    ondemand::document doc;
+    ASSERT_SUCCESS(parser.iterate(json).get(doc));
+    std::string_view view;
+    ASSERT_SUCCESS(doc.get_string().get(view));
+	ASSERT_EQUAL(view, "\xf0\xb2\x80\x80\xf0\x90\x80\x80\xf4\x8f\xbf\xbf");
+    TEST_SUCCEED();
+  }
+
   bool issue1660() {
     TEST_START();
     ondemand::parser parser;
@@ -459,6 +495,9 @@ namespace misc_tests {
   bool run() {
     return
            issue1870() &&
+           issue1894() &&
+           issue1894toolarge() &&
+           issue1894success() &&
            is_alive_root_array() &&
            is_alive_root_object() &&
            is_alive_array() &&