avm2: Fix String.replace crash when regex is used in replacement

(close #17899)

core/src/avm2/globals/string.rs

@@ -5,7 +5,7 @@ use crate::avm2::class::{Class, ClassAttributes};
 use crate::avm2::error::make_error_1004;
 use crate::avm2::method::{Method, NativeMethodImpl};
 use crate::avm2::object::{primitive_allocator, FunctionObject, Object, TObject};
-use crate::avm2::regexp::RegExpFlags;
+use crate::avm2::regexp::{RegExp, RegExpFlags};
 use crate::avm2::value::Value;
 use crate::avm2::Error;
 use crate::avm2::QName;
@@ -355,18 +355,18 @@ fn replace<'gc>(
     let this = Value::from(this).coerce_to_string(activation)?;
     let pattern = args.get(0).unwrap_or(&Value::Undefined);
     let replacement = args.get(1).unwrap_or(&Value::Undefined);
-    // Handles regex patterns.
-    if let Some(mut regexp) = pattern
+
+    if let Some(regexp) = pattern
         .as_object()
         .as_ref()
-        .and_then(|o| o.as_regexp_mut(activation.context.gc_context))
+        .and_then(|o| o.as_regexp_object())
     {
         // Replacement is either a function or treatable as string.
         if let Some(f) = replacement.as_object().and_then(|o| o.as_function_object()) {
-            return Ok(regexp.replace_fn(activation, this, &f)?.into());
+            return Ok(RegExp::replace_fn(regexp, activation, this, &f)?.into());
         } else {
             let replacement = replacement.coerce_to_string(activation)?;
-            return Ok(regexp.replace_string(activation, this, replacement)?.into());
+            return Ok(RegExp::replace_string(regexp, activation, this, replacement)?.into());
         }
     }
 

core/src/avm2/regexp.rs

@@ -13,6 +13,8 @@ use bitflags::bitflags;
 use gc_arena::Collect;
 use ruffle_wstr::WStr;
 
+use super::object::RegExpObject;
+
 #[derive(Collect, Debug)]
 #[collect(no_drop)]
 pub struct RegExp<'gc> {
@@ -214,12 +216,12 @@ impl<'gc> RegExp<'gc> {
     /// Implements string.replace(regex, replacement) where the replacement is
     /// a function.
     pub fn replace_fn(
-        &mut self,
+        regexp: RegExpObject<'gc>,
         activation: &mut Activation<'_, 'gc>,
         text: AvmString<'gc>,
         f: &FunctionObject<'gc>,
     ) -> Result<AvmString<'gc>, Error<'gc>> {
-        self.replace_with_fn(activation, &text, |activation, txt, m| {
+        Self::replace_with_fn(regexp, activation, &text, |activation, txt, m| {
             let args = std::iter::once(Some(&m.range))
                 .chain((m.captures.iter()).map(|x| x.as_ref()))
                 .map(|o| match o {
@@ -241,12 +243,12 @@ impl<'gc> RegExp<'gc> {
     /// Implements string.replace(regex, replacement) where the replacement may be
     /// a string with $-sequences.
     pub fn replace_string(
-        &mut self,
+        regexp: RegExpObject<'gc>,
         activation: &mut Activation<'_, 'gc>,
         text: AvmString<'gc>,
         replacement: AvmString<'gc>,
     ) -> Result<AvmString<'gc>, Error<'gc>> {
-        self.replace_with_fn(activation, &text, |_activation, txt, m| {
+        RegExp::replace_with_fn(regexp, activation, &text, |_activation, txt, m| {
             Ok(Self::effective_replacement(&replacement, txt, m))
         })
     }
@@ -254,8 +256,9 @@ impl<'gc> RegExp<'gc> {
     // Helper for replace_string and replace_function.
     //
     // Replaces occurrences of regex with results of f(activation, &text, &match)
+    // Panics if regexp isn't a regexp
     fn replace_with_fn<'a, F>(
-        &mut self,
+        regexp: RegExpObject<'gc>,
         activation: &mut Activation<'_, 'gc>,
         text: &AvmString<'gc>,
         mut f: F,
@@ -268,8 +271,16 @@ impl<'gc> RegExp<'gc> {
         ) -> Result<Cow<'a, WStr>, Error<'gc>>,
     {
         let mut start = 0;
-        let mut m = self.find_utf16_match(*text, start);
 
+        let (is_global, mut m) = {
+            // we only hold onto a mutable lock on the regular expression
+            // for a small window, because f might refer to the RegExp
+            // (See https://github.com/ruffle-rs/ruffle/issues/17899)
+            let mut re = regexp.as_regexp_mut(activation.gc()).unwrap();
+            let global_flag = re.flags().contains(RegExpFlags::GLOBAL);
+
+            (global_flag, re.find_utf16_match(*text, start))
+        };
         if m.is_none() {
             // Nothing to do; short circuit and just return the original string, to avoid any allocs or functions
             return Ok(*text);
@@ -290,10 +301,16 @@ impl<'gc> RegExp<'gc> {
                 start += 1;
             }
 
-            if !self.flags().contains(RegExpFlags::GLOBAL) {
+            if !is_global {
                 break;
             }
-            m = self.find_utf16_match(*text, start);
+            // Again, here we only hold onto a mutable lock for
+            // the RegExp long enough to do our matching, so that
+            // when we call f we don't have a lock
+            m = regexp
+                .as_regexp_mut(activation.gc())
+                .unwrap()
+                .find_utf16_match(*text, start);
         }
 
         ret.push_str(&text[start..]);
@@ -347,7 +364,11 @@ impl<'gc> RegExp<'gc> {
         ArrayObject::from_storage(activation, storage)
     }
 
-    fn find_utf16_match(&mut self, text: AvmString<'gc>, start: usize) -> Option<regress::Match> {
+    pub fn find_utf16_match(
+        &mut self,
+        text: AvmString<'gc>,
+        start: usize,
+    ) -> Option<regress::Match> {
         self.find_utf8_match_at(text, start, |text, mut re_match| {
             // Sort the capture endpoints by increasing index, so that CachedText::utf16_index is efficient.
             let mut utf8_indices = re_match

tests/tests/swfs/avm2/string_replace/Test.as

@@ -93,3 +93,10 @@ trace("<<a>>".replace(/(a)(b)?|(c)/, rFN))
 
 // The pattern is string and the replacement is a function
 trace("<<a>>".replace("a", rFN))
+
+trace("// regex calling into itself")
+
+var pattern = /simple/g
+trace("this is simple, really simple.".replace(pattern, function (match) {
+  return match.replace(pattern, "complicated")
+}))

tests/tests/swfs/avm2/string_replace/output.txt

@@ -39,3 +39,5 @@ undefinedbbbb
 // replace a regex with function, check arguments
 <<a,a,,,2,<<a>>>>
 <<a,2,<<a>>>>
+// regex calling into itself
+this is complicated, really complicated.