webkit2gtk3/SOURCES/CVE-2023-28204.patch

From 8efa99e7b5d5a37aefb476cc27ee24c2be4da0c7 Mon Sep 17 00:00:00 2001
From: Michael Saboff <msaboff@apple.com>
Date: Mon, 22 May 2023 13:40:46 -0700
Subject: [PATCH] Cherry-pick 264365@main (698c6e293734).
 https://bugs.webkit.org/show_bug.cgi?id=254930

    [JSC] RegExpGlobalData::performMatch issue leading to OOB read
    https://bugs.webkit.org/show_bug.cgi?id=254930
    rdar://107436732

    Reviewed by Alexey Shvayka.

    Fixed two issues:
    1) In YarrInterpreter.cpp::matchAssertionBOL() we were advancing the string position for non-BMP
       characters.  Since it is an assertion, we shouldn't advance the character position.
       Made the same fix to matchAssertionEOL().
    2) In StringPrototype.cpp::replaceUsingRegExpSearch(), we need to advance past both elements of
       a non-BMP character for the case where the RegExp match is empty.

    * JSTests/stress/string-replace-regexp-matchBOL-correct-advancing.js: New test.
    * Source/JavaScriptCore/runtime/StringPrototype.cpp:
    (JSC::replaceUsingRegExpSearch):
    * Source/JavaScriptCore/yarr/YarrInterpreter.cpp:
    (JSC::Yarr::Interpreter::InputStream::readCheckedDontAdvance):
    (JSC::Yarr::Interpreter::matchAssertionBOL):
    (JSC::Yarr::Interpreter::matchAssertionEOL):

    Originally-landed-as: 259548.551@safari-7615-branch (e34edaa74575). rdar://107436732
    Canonical link: https://commits.webkit.org/264365@main
---
 ...place-regexp-matchBOL-correct-advancing.js | 35 ++++++++++++++++++
 .../runtime/StringPrototype.cpp               | 10 ++++++
 .../JavaScriptCore/yarr/YarrInterpreter.cpp   | 36 +++++++++++++++++--
 3 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 JSTests/stress/string-replace-regexp-matchBOL-correct-advancing.js

diff --git a/JSTests/stress/string-replace-regexp-matchBOL-correct-advancing.js b/JSTests/stress/string-replace-regexp-matchBOL-correct-advancing.js
new file mode 100644
index 000000000000..25b1a70b81d2
--- /dev/null
+++ b/JSTests/stress/string-replace-regexp-matchBOL-correct-advancing.js
@@ -0,0 +1,35 @@
+// Check that we don't advance for BOL assertions when matching a non-BMP character in the YARR interpreter
+// and that we do advance in String.replace() when processing an empty match.
+
+let expected = "|";
+
+for (let i = 0; i < 11; ++i)
+    expected += String.fromCodePoint(128512) + '|';
+
+let str = String.fromCodePoint(128512).repeat(11);
+
+let result1 = str.replace(/(?!(?=^a|()+()+x)(abc))/gmu, r => {
+    return '|';
+});
+
+
+if (result1 !== expected)
+    print("FAILED: \"" + result1 + " !== " + expected + '"');
+
+let result2= str.replace(/(?!(?=^a|x)(abc))/gmu, r => {
+    return '|';
+});
+
+if (result2 !== expected)
+    print("FAILED: \"" + result2 + " !== " + expected + '"');
+
+expected = "|" + String.fromCodePoint(128512);
+
+str = String.fromCodePoint(128512).repeat(1);
+
+let result3= str.replace(/(?!(?=^a|x)(abc))/mu, r => {
+    return '|';
+});
+
+if (result3 !== expected)
+    print("FAILED: \"" + result3 + " !== " + expected + '"');
diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp
index 08104b1dbfa9..459295f728a7 100644
--- a/Source/JavaScriptCore/runtime/StringPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp
@@ -603,6 +603,11 @@ static ALWAYS_INLINE JSString* replaceUsingRegExpSearch(
                 startPosition++;
                 if (startPosition > sourceLen)
                     break;
+                if (U16_IS_LEAD(source[startPosition - 1]) && U16_IS_TRAIL(source[startPosition])) {
+                    startPosition++;
+                    if (startPosition > sourceLen)
+                        break;
+                }
             }
         }
     } else {
@@ -682,6 +687,11 @@ static ALWAYS_INLINE JSString* replaceUsingRegExpSearch(
                 startPosition++;
                 if (startPosition > sourceLen)
                     break;
+                if (U16_IS_LEAD(source[startPosition - 1]) && U16_IS_TRAIL(source[startPosition])) {
+                    startPosition++;
+                    if (startPosition > sourceLen)
+                        break;
+                }
             }
         } while (global);
     }
diff --git a/Source/JavaScriptCore/yarr/YarrInterpreter.cpp b/Source/JavaScriptCore/yarr/YarrInterpreter.cpp
index 95a848a1a66d..b1a22b253866 100644
--- a/Source/JavaScriptCore/yarr/YarrInterpreter.cpp
+++ b/Source/JavaScriptCore/yarr/YarrInterpreter.cpp
@@ -209,6 +209,38 @@ public:
             }
             return result;
         }
+
+        int readCheckedDontAdvance(unsigned negativePositionOffest)
+        {
+            RELEASE_ASSERT(pos >= negativePositionOffest);
+            unsigned p = pos - negativePositionOffest;
+            ASSERT(p < length);
+            int result = input[p];
+            if (U16_IS_LEAD(result) && decodeSurrogatePairs && p + 1 < length && U16_IS_TRAIL(input[p + 1])) {
+                if (atEnd())
+                    return -1;
+
+                result = U16_GET_SUPPLEMENTARY(result, input[p + 1]);
+            }
+            return result;
+        }
+
+        // readForCharacterDump() is only for use by the DUMP_CURR_CHAR macro.
+        // We don't want any side effects like the next() in readChecked() above.
+        int readForCharacterDump(unsigned negativePositionOffest)
+        {
+            RELEASE_ASSERT(pos >= negativePositionOffest);
+            unsigned p = pos - negativePositionOffest;
+            ASSERT(p < length);
+            int result = input[p];
+            if (U16_IS_LEAD(result) && decodeSurrogatePairs && p + 1 < length && U16_IS_TRAIL(input[p + 1])) {
+                if (atEnd())
+                    return -1;
+
+                result = U16_GET_SUPPLEMENTARY(result, input[p + 1]);
+            }
+            return result;
+        }
         
         int readSurrogatePairChecked(unsigned negativePositionOffset)
         {
@@ -482,13 +514,13 @@ public:
 
     bool matchAssertionBOL(ByteTerm& term)
     {
-        return (input.atStart(term.inputPosition)) || (pattern->multiline() && testCharacterClass(pattern->newlineCharacterClass, input.readChecked(term.inputPosition + 1)));
+        return (input.atStart(term.inputPosition)) || (pattern->multiline() && testCharacterClass(pattern->newlineCharacterClass, input.readCheckedDontAdvance(term.inputPosition + 1)));
     }
 
     bool matchAssertionEOL(ByteTerm& term)
     {
         if (term.inputPosition)
-            return (input.atEnd(term.inputPosition)) || (pattern->multiline() && testCharacterClass(pattern->newlineCharacterClass, input.readChecked(term.inputPosition)));
+            return (input.atEnd(term.inputPosition)) || (pattern->multiline() && testCharacterClass(pattern->newlineCharacterClass, input.readCheckedDontAdvance(term.inputPosition)));
 
         return (input.atEnd()) || (pattern->multiline() && testCharacterClass(pattern->newlineCharacterClass, input.read()));
     }
-- 
2.40.1