fix: process issues from external security review

nova_vm/src/ecmascript/abstract_operations/operations_on_iterator_objects.rs

@@ -25,7 +25,7 @@ use crate::{
     },
     engine::{
         ScopableCollection, ScopedCollection, VmIteratorRecord,
-        context::{Bindable, GcScope, bindable_handle},
+        context::{Bindable, GcScope, NoGcScope, bindable_handle},
         rootable::Scopable,
     },
     heap::{
@@ -609,7 +609,8 @@ pub(crate) fn create_iter_result_object<'a>(
     agent: &mut Agent,
     value: Value<'a>,
     done: bool,
-) -> OrdinaryObject<'a> {
+    gc: NoGcScope<'a, '_>,
+) -> JsResult<'a, OrdinaryObject<'a>> {
     // 1. Let obj be OrdinaryObjectCreate(%Object.prototype%).
     // 2. Perform ! CreateDataPropertyOrThrow(obj, "value", value).
     // 3. Perform ! CreateDataPropertyOrThrow(obj, "done", done).
@@ -644,6 +645,7 @@ pub(crate) fn create_iter_result_object<'a>(
             },
         ],
     )
+    .map_err(|err| agent.throw_allocation_exception(err, gc))
 }
 
 /// ### [7.4.16 IteratorToList ( iteratorRecord )](https://tc39.es/ecma262/#sec-iteratortolist)

nova_vm/src/ecmascript/abstract_operations/operations_on_objects.rs

@@ -2248,7 +2248,7 @@ pub(crate) fn try_copy_data_properties_into_object<'gc, 'b>(
         }
     }
 
-    TryResult::Continue(OrdinaryObject::create_object(
+    let obj = OrdinaryObject::create_object(
         agent,
         Some(
             agent
@@ -2258,7 +2258,13 @@ pub(crate) fn try_copy_data_properties_into_object<'gc, 'b>(
                 .into_object(),
         ),
         &entries,
-    ))
+    );
+    if let Ok(obj) = obj {
+        TryResult::Continue(obj)
+    } else {
+        // We failed to allocate: try run GC.
+        TryError::GcError.into()
+    }
 }
 
 /// ### [7.3.25 CopyDataProperties ( target, source, excludedItems )](https://tc39.es/ecma262/#sec-copydataproperties)
@@ -2327,7 +2333,7 @@ pub(crate) fn copy_data_properties_into_object<'a, 'b>(
         i += 1;
     }
 
-    let object = OrdinaryObject::create_object(
+    let object = match OrdinaryObject::create_object(
         agent,
         Some(
             agent
@@ -2337,8 +2343,10 @@ pub(crate) fn copy_data_properties_into_object<'a, 'b>(
                 .into_object(),
         ),
         &entries,
-    )
-    .bind(gc.nogc());
+    ) {
+        Ok(obj) => obj,
+        Err(err) => return Err(agent.throw_allocation_exception(err, gc.into_nogc())),
+    };
 
     if broke {
         let _ = keys.drain(..i);

nova_vm/src/ecmascript/builders/ordinary_object_builder.rs

@@ -361,7 +361,8 @@ pub(super) fn create_intrinsic_backing_object(
     let (cap, index) = agent
         .heap
         .elements
-        .allocate_keys_with_capacity(properties_count);
+        .allocate_keys_with_capacity(properties_count)
+        .expect("Intrinsic object's property allocation failed");
     let cap = cap.make_intrinsic();
     let keys_memory = agent.heap.elements.get_keys_uninit_raw(cap, index);
     for (slot, key) in keys_memory.iter_mut().zip(properties.iter().map(|e| e.0)) {

nova_vm/src/ecmascript/builtins/arguments.rs

@@ -26,6 +26,8 @@
 //!
 //! ECMAScript implementations of arguments exotic objects have historically contained an accessor property named "caller". Prior to ECMAScript 2017, this specification included the definition of a throwing "caller" property on ordinary arguments objects. Since implementations do not contain this extension any longer, ECMAScript 2017 dropped the requirement for a throwing "caller" accessor.
 
+use std::collections::TryReserveError;
+
 use ahash::AHashMap;
 
 use crate::{
@@ -124,7 +126,7 @@ pub(crate) fn create_unmapped_arguments_object<'a, 'b>(
     agent: &mut Agent,
     arguments_list: &ScopedArgumentsList<'b>,
     gc: NoGcScope<'a, 'b>,
-) -> Object<'a> {
+) -> Result<Object<'a>, TryReserveError> {
     // 1. Let len be the number of elements in argumentsList.
     let len = arguments_list.len(agent);
     // SAFETY: GC is not allowed in this scope, and no other scoped values are
@@ -136,11 +138,11 @@ pub(crate) fn create_unmapped_arguments_object<'a, 'b>(
     // 2. Let obj be OrdinaryObjectCreate(%Object.prototype%, « [[ParameterMap]] »).
     let prototype = agent.current_realm_record().intrinsics().object_prototype();
     let mut shape = ObjectShape::get_shape_for_prototype(agent, Some(prototype.into_object()));
-    shape = shape.get_child_shape(agent, BUILTIN_STRING_MEMORY.length.to_property_key());
-    shape = shape.get_child_shape(agent, BUILTIN_STRING_MEMORY.callee.into());
-    shape = shape.get_child_shape(agent, WellKnownSymbolIndexes::Iterator.into());
+    shape = shape.get_child_shape(agent, BUILTIN_STRING_MEMORY.length.to_property_key())?;
+    shape = shape.get_child_shape(agent, BUILTIN_STRING_MEMORY.callee.into())?;
+    shape = shape.get_child_shape(agent, WellKnownSymbolIndexes::Iterator.into())?;
     for index in 0..len {
-        shape = shape.get_child_shape(agent, index.into());
+        shape = shape.get_child_shape(agent, index.into())?;
     }
     let obj = OrdinaryObject::create_object_with_shape(agent, shape)
         .expect("Failed to create Arguments object storage");
@@ -213,7 +215,7 @@ pub(crate) fn create_unmapped_arguments_object<'a, 'b>(
     // [[Configurable]]: false
     // }).
     // 9. Return obj.
-    Object::Arguments(obj)
+    Ok(Object::Arguments(obj))
 }
 
 // 10.4.4.7 CreateMappedArgumentsObject ( func, formals, argumentsList, env )

nova_vm/src/ecmascript/builtins/array/abstract_operations.rs

@@ -63,7 +63,11 @@ pub(crate) fn array_create<'a>(
         {
             None
         } else {
-            Some(OrdinaryObject::create_object(agent, Some(proto), &[]).bind(gc))
+            Some(
+                OrdinaryObject::create_object(agent, Some(proto), &[])
+                    .expect("Should perform GC here")
+                    .bind(gc),
+            )
         }
     } else {
         None

nova_vm/src/ecmascript/builtins/bound_function.rs

@@ -169,6 +169,7 @@ impl<'a> FunctionInternalProperties<'a> for BoundFunction<'a> {
         arguments_list: ArgumentsList,
         gc: GcScope<'gc, '_>,
     ) -> JsResult<'gc, Value<'gc>> {
+        agent.check_call_depth(gc.nogc()).unbind()?;
         let f = self.bind(gc.nogc());
         let arguments_list = arguments_list.bind(gc.nogc());
         // 1. Let target be F.[[BoundTargetFunction]].
@@ -219,6 +220,7 @@ impl<'a> FunctionInternalProperties<'a> for BoundFunction<'a> {
         new_target: Function,
         gc: GcScope<'gc, '_>,
     ) -> JsResult<'gc, Object<'gc>> {
+        agent.check_call_depth(gc.nogc()).unbind()?;
         let arguments_list = arguments_list.bind(gc.nogc());
         let new_target = new_target.bind(gc.nogc());
         // 1. Let target be F.[[BoundTargetFunction]].

nova_vm/src/ecmascript/builtins/builtin_constructor.rs

@@ -351,7 +351,8 @@ pub(crate) fn create_builtin_constructor<'a>(
         },
     };
     let entries = [length_entry, name_entry, prototype_entry];
-    let backing_object = OrdinaryObject::create_intrinsic_object(agent, args.prototype, &entries);
+    let backing_object = OrdinaryObject::create_intrinsic_object(agent, args.prototype, &entries)
+        .expect("Should perform GC here");
 
     // 13. Return func.
     agent

nova_vm/src/ecmascript/builtins/builtin_function.rs

@@ -665,6 +665,8 @@ fn builtin_call_or_construct<'gc>(
     new_target: Option<Function>,
     gc: GcScope<'gc, '_>,
 ) -> JsResult<'gc, Value<'gc>> {
+    agent.check_call_depth(gc.nogc()).unbind()?;
+
     let f = f.bind(gc.nogc());
     let this_argument = this_argument.bind(gc.nogc());
     let arguments_list = arguments_list.bind(gc.nogc());
@@ -812,11 +814,10 @@ pub fn create_builtin_function<'a>(
                     configurable: true,
                 },
             };
-            Some(OrdinaryObject::create_object(
-                agent,
-                Some(prototype),
-                &[length_entry, name_entry],
-            ))
+            Some(
+                OrdinaryObject::create_object(agent, Some(prototype), &[length_entry, name_entry])
+                    .expect("Should perform GC here"),
+            )
         }
     } else {
         None

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_abstract_operations.rs

@@ -127,7 +127,8 @@ fn async_generator_complete_step(
             agent.set_current_realm(realm);
         }
         // iii. Let iteratorResult be CreateIteratorResultObject(value, done).
-        let iterator_result = create_iter_result_object(agent, value, done);
+        let iterator_result =
+            create_iter_result_object(agent, value, done, gc).expect("Should perform GC here");
         // iv. Set the running execution context's Realm to oldRealm.
         if set_realm {
             agent.set_current_realm(old_realm);
@@ -136,7 +137,7 @@ fn async_generator_complete_step(
     } else {
         // c. Else,
         // i. Let iteratorResult be CreateIteratorResultObject(value, done).
-        create_iter_result_object(agent, value, done)
+        create_iter_result_object(agent, value, done, gc).expect("Should perform GC here")
     };
     // d. Perform ! Call(promiseCapability.[[Resolve]], undefined, « iteratorResult »).
     unwrap_try(promise_capability.try_resolve(agent, iterator_result.into_value(), gc));

nova_vm/src/ecmascript/builtins/control_abstraction_objects/async_generator_objects/async_generator_prototype.rs

@@ -86,7 +86,10 @@ impl AsyncGeneratorPrototype {
         // 6. If state is completed, then
         if state.is_completed() {
             // a. Let iteratorResult be CreateIteratorResultObject(undefined, true).
-            let iterator_result = create_iter_result_object(agent, Value::Undefined, true);
+            let iterator_result =
+                create_iter_result_object(agent, Value::Undefined, true, gc.nogc())
+                    .unbind()?
+                    .bind(gc.nogc());
             // b. Perform ! Call(promiseCapability.[[Resolve]], undefined, « iteratorResult »).
             promise_capability
                 .unbind()

nova_vm/src/ecmascript/builtins/control_abstraction_objects/generator_objects.rs

@@ -57,7 +57,8 @@ impl Generator<'_> {
             }
             GeneratorState::Completed => {
                 // 2. If state is completed, return CreateIterResultObject(undefined, true).
-                return Ok(create_iter_result_object(agent, Value::Undefined, true).into_value());
+                return create_iter_result_object(agent, Value::Undefined, true, gc.into_nogc())
+                    .map(|o| o.into_value());
             }
             GeneratorState::SuspendedStart(_) | GeneratorState::SuspendedYield(_) => {
                 // 3. Assert: state is either suspended-start or suspended-yield.
@@ -121,7 +122,8 @@ impl Generator<'_> {
                 // j. Else if result is a return completion, then
                 //    i. Let resultValue be result.[[Value]].
                 // l. Return CreateIterResultObject(resultValue, true).
-                Ok(create_iter_result_object(agent, result_value, true).into_value())
+                create_iter_result_object(agent, result_value, true, gc.into_nogc())
+                    .map(|o| o.into_value())
             }
             ExecutionResult::Throw(err) => {
                 // GeneratorStart step 4:
@@ -243,7 +245,8 @@ impl Generator<'_> {
         match execution_result {
             ExecutionResult::Return(result) => {
                 agent[generator].generator_state = Some(GeneratorState::Completed);
-                Ok(create_iter_result_object(agent, result.unbind(), true).into_value())
+                create_iter_result_object(agent, result.unbind(), true, gc.into_nogc())
+                    .map(|o| o.into_value())
             }
             ExecutionResult::Throw(err) => {
                 agent[generator].generator_state = Some(GeneratorState::Completed);
@@ -285,9 +288,13 @@ impl Generator<'_> {
 
                 // 3. If abruptCompletion is a return completion, then
                 // i. Return CreateIteratorResultObject(abruptCompletion.[[Value]], true).
-                return Ok(
-                    create_iter_result_object(agent, abrupt_completion.unbind(), true).into_value(),
-                );
+                return create_iter_result_object(
+                    agent,
+                    abrupt_completion.unbind(),
+                    true,
+                    gc.into_nogc(),
+                )
+                .map(|o| o.into_value());
             }
             GeneratorState::SuspendedYield(_) => {
                 // 4. Assert: state is suspended-yield.
@@ -302,9 +309,13 @@ impl Generator<'_> {
             GeneratorState::Completed => {
                 // 3. If abruptCompletion is a return completion, then
                 // i. Return CreateIteratorResultObject(abruptCompletion.[[Value]], true).
-                return Ok(
-                    create_iter_result_object(agent, abrupt_completion.unbind(), true).into_value(),
-                );
+                return create_iter_result_object(
+                    agent,
+                    abrupt_completion.unbind(),
+                    true,
+                    gc.into_nogc(),
+                )
+                .map(|o| o.into_value());
             }
         };
 
@@ -362,7 +373,8 @@ impl Generator<'_> {
         match execution_result {
             ExecutionResult::Return(result) => {
                 agent[generator].generator_state = Some(GeneratorState::Completed);
-                Ok(create_iter_result_object(agent, result, true).into_value())
+                create_iter_result_object(agent, result, true, gc.into_nogc())
+                    .map(|o| o.into_value())
             }
             ExecutionResult::Throw(err) => {
                 agent[generator].generator_state = Some(GeneratorState::Completed);

nova_vm/src/ecmascript/builtins/control_abstraction_objects/iteration/async_from_sync_iterator_objects.rs

@@ -157,8 +157,13 @@ impl AsyncFromSyncIteratorPrototype {
         // 8. If return is undefined, then
         let Some(r#return) = r#return else {
             // a. Let iteratorResult be CreateIteratorResultObject(value, true).
-            let iterator_result =
-                create_iter_result_object(agent, value.unwrap_or(Value::Undefined), true);
+            let iterator_result = create_iter_result_object(
+                agent,
+                value.unwrap_or(Value::Undefined),
+                true,
+                gc.nogc(),
+            )
+            .expect("Should perform GC here");
             // b. Perform ! Call(promiseCapability.[[Resolve]], undefined, « iteratorResult »).
             unwrap_try(promise_capability.try_resolve(
                 agent,

nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_abstract_operations/promise_finally_functions.rs

@@ -159,6 +159,7 @@ impl<'a> FunctionInternalProperties<'a> for BuiltinPromiseFinallyFunction<'a> {
         arguments_list: ArgumentsList,
         mut gc: GcScope<'gc, '_>,
     ) -> JsResult<'gc, Value<'gc>> {
+        agent.check_call_depth(gc.nogc()).unbind()?;
         let f = self.bind(gc.nogc());
         match f.get(agent).resolve_type {
             PromiseFinallyFunctionType::ResolveFinally { on_finally, c } => {

nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_abstract_operations/promise_group_record.rs

@@ -209,6 +209,7 @@ impl<'a> PromiseGroup<'a> {
             ),
             &entries,
         )
+        .expect("Should perform GC here")
         .bind(gc);
 
         obj.into_value().unbind()

nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_abstract_operations/promise_jobs.rs

@@ -208,7 +208,8 @@ impl PromiseReactionJob {
                 //    called:
                 // a. Return CreateIteratorResultObject(v, done).
                 (
-                    Ok(create_iter_result_object(agent, argument, done).into_value()),
+                    create_iter_result_object(agent, argument, done, gc.nogc())
+                        .map(|o| o.into_value()),
                     capability,
                 )
             }

nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_abstract_operations/promise_resolving_functions.rs

@@ -108,6 +108,7 @@ impl<'a> FunctionInternalProperties<'a> for BuiltinPromiseResolvingFunction<'a>
         arguments_list: ArgumentsList,
         gc: GcScope<'gc, '_>,
     ) -> JsResult<'gc, Value<'gc>> {
+        agent.check_call_depth(gc.nogc()).unbind()?;
         let arguments_list = arguments_list.get(0).bind(gc.nogc());
         let promise_capability = agent[self].promise_capability.clone();
         match agent[self].resolve_type {

nova_vm/src/ecmascript/builtins/control_abstraction_objects/promise_objects/promise_constructor.rs

@@ -713,7 +713,8 @@ impl PromiseConstructor {
                     reject_function.into_value(),
                 ),
             ],
-        );
+        )
+        .expect("Should perform GC here");
 
         // 7. Return obj.
         Ok(obj.into_value())

nova_vm/src/ecmascript/builtins/ecmascript_function.rs

@@ -330,6 +330,7 @@ impl<'a> FunctionInternalProperties<'a> for ECMAScriptFunction<'a> {
         arguments_list: ArgumentsList,
         gc: GcScope<'gc, '_>,
     ) -> JsResult<'gc, Value<'gc>> {
+        agent.check_call_depth(gc.nogc()).unbind()?;
         let f = self.bind(gc.nogc());
         let mut id = 0;
         ndt::javascript_call_start!(|| {
@@ -835,11 +836,10 @@ pub(crate) fn ordinary_function_create<'agent, 'program, 'gc>(
                 .function_prototype()
                 .into_object()
     {
-        function.object_index = Some(OrdinaryObject::create_object(
-            agent,
-            Some(function_prototype),
-            &[],
-        ));
+        function.object_index = Some(
+            OrdinaryObject::create_object(agent, Some(function_prototype), &[])
+                .expect("Should perform GC here"),
+        );
     }
 
     // 18. Set F.[[Fields]] to a new empty List.

nova_vm/src/ecmascript/builtins/error.rs

@@ -131,7 +131,8 @@ impl<'a> InternalSlots<'a> for Error<'a> {
                 OrdinaryObject::create_object(agent, Some(prototype), &[cause_entry])
             } else {
                 OrdinaryObject::create_object(agent, Some(prototype), &[])
-            };
+            }
+            .expect("Should perform GC here");
         self.set_backing_object(agent, backing_object);
         backing_object
     }

nova_vm/src/ecmascript/builtins/fundamental_objects/function_objects/function_prototype.rs

@@ -560,7 +560,8 @@ fn create_throw_type_error_backing_object(
             configurable: false,
         },
     };
-    let object = OrdinaryObject::create_object(agent, Some(prototype), &[length_entry, name_entry]);
+    let object = OrdinaryObject::create_object(agent, Some(prototype), &[length_entry, name_entry])
+        .expect("Should perform GC here");
     // The value of the [[Extensible]] internal slot of this function is false.
     object.internal_set_extensible(agent, false);
     object