fix(string): add maximum length limit to JsString to prevent OOM crashes

core/engine/src/builtins/string/mod.rs

@@ -659,8 +659,10 @@ impl String {
         // 4. For each element next of args, do
         for arg in args {
             // a. Let nextString be ? ToString(next).
+            let next_string = arg.to_string(context)?;
             // b. Set R to the string-concatenation of R and nextString.
-            string = js_string!(&string, &arg.to_string(context)?);
+            string = JsString::concat(string.as_str(), next_string.as_str())
+                .map_err(|_| JsNativeError::range().with_message("Invalid string length"))?;
         }
 
         // 5. Return R.
@@ -741,7 +743,9 @@ impl String {
         }
 
         // 6. Return the String value that is made from n copies of S appended together.
-        Ok(JsString::concat_array(&result).into())
+        Ok(JsString::concat_array(&result)
+            .map_err(|_| JsNativeError::range().with_message("Invalid string length"))?
+            .into())
     }
 
     /// `String.prototype.slice( beginIndex [, endIndex] )`

core/engine/src/builtins/string/tests.rs

@@ -975,3 +975,20 @@ fn match_with_overridden_exec() {
         js_str!("fake"),
     )]);
 }
+
+#[test]
+fn concat_max_length_overflow() {
+    // Test for issue #4409: Repeated string concatenation should throw RangeError
+    // instead of causing OOM crash
+    run_test_actions([TestAction::assert_native_error(
+        indoc! {r"
+            var s = '\u1234--synchronized-----';
+            for (var i = 0; i < 17; i++) {
+                s += s; 
+                s += s;
+            }
+        "},
+        JsNativeErrorKind::Range,
+        "Invalid string length",
+    )]);
+}

core/engine/src/string.rs

@@ -62,10 +62,10 @@ macro_rules! js_string {
         $crate::string::JsString::from($s)
     };
     ( $x:expr, $y:expr ) => {
-        $crate::string::JsString::concat($crate::string::JsStr::from($x), $crate::string::JsStr::from($y))
+        $crate::string::JsString::concat($crate::string::JsStr::from($x), $crate::string::JsStr::from($y)).expect("string concatenation overflow")
     };
     ( $( $s:expr ),+ ) => {
-        $crate::string::JsString::concat_array(&[ $( $crate::string::JsStr::from($s) ),+ ])
+        $crate::string::JsString::concat_array(&[ $( $crate::string::JsStr::from($s) ),+ ]).expect("string concatenation overflow")
     };
 }
 

core/engine/src/value/operations.rs

@@ -1,11 +1,10 @@
 use crate::{
-    Context, JsBigInt, JsResult, JsValue, JsVariant,
+    Context, JsBigInt, JsResult, JsString, JsValue, JsVariant,
     builtins::{
         Number,
         number::{f64_to_int32, f64_to_uint32},
     },
     error::JsNativeError,
-    js_string,
     value::{JsSymbol, Numeric, PreferredType},
 };
 
@@ -24,15 +23,33 @@ impl JsValue {
             (JsVariant::BigInt(x), JsVariant::BigInt(y)) => Self::new(JsBigInt::add(&x, &y)),
 
             // String concat
-            (JsVariant::String(x), JsVariant::String(y)) => Self::from(js_string!(&x, &y)),
+            (JsVariant::String(x), JsVariant::String(y)) => {
+                let result = JsString::concat(x.as_str(), y.as_str())
+                    .map_err(|_| JsNativeError::range().with_message("Invalid string length"))?;
+                Self::from(result)
+            }
 
             // Slow path:
             (_, _) => {
                 let x = self.to_primitive(context, PreferredType::Default)?;
                 let y = other.to_primitive(context, PreferredType::Default)?;
                 match (x.variant(), y.variant()) {
-                    (JsVariant::String(x), _) => Self::from(js_string!(&x, &y.to_string(context)?)),
-                    (_, JsVariant::String(y)) => Self::from(js_string!(&x.to_string(context)?, &y)),
+                    (JsVariant::String(x), _) => {
+                        let y_str = y.to_string(context)?;
+                        let result =
+                            JsString::concat(x.as_str(), y_str.as_str()).map_err(|_| {
+                                JsNativeError::range().with_message("Invalid string length")
+                            })?;
+                        Self::from(result)
+                    }
+                    (_, JsVariant::String(y)) => {
+                        let x_str = x.to_string(context)?;
+                        let result =
+                            JsString::concat(x_str.as_str(), y.as_str()).map_err(|_| {
+                                JsNativeError::range().with_message("Invalid string length")
+                            })?;
+                        Self::from(result)
+                    }
                     (_, _) => {
                         match (x.to_numeric(context)?, y.to_numeric(context)?) {
                             (Numeric::Number(x), Numeric::Number(y)) => Self::new(x + y),

core/engine/src/vm/opcode/concat/mod.rs

@@ -21,7 +21,10 @@ impl ConcatToString {
             let val = context.vm.get_register(value.into()).clone();
             strings.push(val.to_string(context)?);
         }
-        let s = JsString::concat_array(&strings.iter().map(JsString::as_str).collect::<Vec<_>>());
+        let s = JsString::concat_array(&strings.iter().map(JsString::as_str).collect::<Vec<_>>())
+            .map_err(|_| {
+            crate::error::JsNativeError::range().with_message("Invalid string length")
+        })?;
         context.vm.set_register(string.into(), s.into());
         Ok(())
     }

core/string/src/lib.rs

@@ -48,6 +48,10 @@ use std::{
 };
 use vtable::JsStringVTable;
 
+/// Maximum string length allowed (`u32::MAX`).
+/// This prevents OOM crashes from exponential string growth during concatenation.
+pub const MAX_STRING_LENGTH: usize = u32::MAX as usize;
+
 fn alloc_overflow() -> ! {
     panic!("detected overflow during string allocation")
 }
@@ -628,29 +632,45 @@ impl JsString {
     }
 
     /// Creates a new [`JsString`] from the concatenation of `x` and `y`.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the resulting string would exceed [`MAX_STRING_LENGTH`].
     #[inline]
-    #[must_use]
-    pub fn concat(x: JsStr<'_>, y: JsStr<'_>) -> Self {
+    pub fn concat(x: JsStr<'_>, y: JsStr<'_>) -> Result<Self, &'static str> {
         Self::concat_array(&[x, y])
     }
 
     /// Creates a new [`JsString`] from the concatenation of every element of
     /// `strings`.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the resulting string would exceed [`MAX_STRING_LENGTH`].
     #[inline]
-    #[must_use]
-    pub fn concat_array(strings: &[JsStr<'_>]) -> Self {
+    pub fn concat_array(strings: &[JsStr<'_>]) -> Result<Self, &'static str> {
         let mut latin1_encoding = true;
         let mut full_count = 0usize;
         for string in strings {
             let Some(sum) = full_count.checked_add(string.len()) else {
-                alloc_overflow()
+                return Err("Invalid string length");
             };
+            // Check if the resulting string would exceed the maximum length
+            if sum > MAX_STRING_LENGTH {
+                return Err("Invalid string length");
+            }
             if !string.is_latin1() {
                 latin1_encoding = false;
             }
             full_count = sum;
         }
 
+        // For UTF-16 strings, also check that the byte count doesn't overflow usize
+        // (each character is 2 bytes). This is important for 32-bit systems.
+        if !latin1_encoding && full_count.checked_mul(2).is_none() {
+            return Err("Invalid string length");
+        }
+
         let (ptr, data_offset) = if latin1_encoding {
             let p = SequenceString::<Latin1>::allocate(full_count);
             (p.cast::<u8>(), size_of::<SequenceString<Latin1>>())
@@ -707,7 +727,7 @@ impl JsString {
             Self { ptr: ptr.cast() }
         };
 
-        StaticJsStrings::get_string(&string.as_str()).unwrap_or(string)
+        Ok(StaticJsStrings::get_string(&string.as_str()).unwrap_or(string))
     }
 
     /// Creates a new [`JsString`] from `data`, without checking if the string is in the interner.
@@ -853,13 +873,15 @@ impl From<&[JsString]> for JsString {
     #[inline]
     fn from(value: &[JsString]) -> Self {
         Self::concat_array(&value.iter().map(Self::as_str).collect::<Vec<_>>()[..])
+            .expect("string concatenation overflow")
     }
 }
 
 impl<const N: usize> From<&[JsString; N]> for JsString {
     #[inline]
     fn from(value: &[JsString; N]) -> Self {
         Self::concat_array(&value.iter().map(Self::as_str).collect::<Vec<_>>()[..])
+            .expect("string concatenation overflow")
     }
 }
 

core/string/src/tests.rs

@@ -127,15 +127,15 @@ fn concat() {
     let x = JsString::from("hello");
     let z = JsString::from("world");
 
-    let xy = JsString::concat(x.as_str(), JsString::from(Y).as_str());
+    let xy = JsString::concat(x.as_str(), JsString::from(Y).as_str()).unwrap();
     assert_eq!(&xy, &ascii_to_utf16(b"hello, "));
     assert_eq!(xy.refcount(), Some(1));
 
-    let xyz = JsString::concat(xy.as_str(), z.as_str());
+    let xyz = JsString::concat(xy.as_str(), z.as_str()).unwrap();
     assert_eq!(&xyz, &ascii_to_utf16(b"hello, world"));
     assert_eq!(xyz.refcount(), Some(1));
 
-    let xyzw = JsString::concat(xyz.as_str(), JsString::from(W).as_str());
+    let xyzw = JsString::concat(xyz.as_str(), JsString::from(W).as_str()).unwrap();
     assert_eq!(&xyzw, &ascii_to_utf16(b"hello, world!"));
     assert_eq!(xyzw.refcount(), Some(1));
 }