detect re-entrent upgrade in syscall

Author: fridrik01

fvm/src/call_manager/default.rs

@@ -14,6 +14,7 @@ use fvm_shared::event::StampedEvent;
 use fvm_shared::sys::BlockId;
 use fvm_shared::{ActorID, METHOD_SEND};
 use num_traits::Zero;
+use std::collections::HashMap;
 
 use super::state_access_tracker::{ActorAccessState, StateAccessTracker};
 use super::{Backtrace, CallManager, Entrypoint, InvocationResult, NO_DATA_BLOCK_ID};
@@ -75,6 +76,7 @@ pub struct InnerDefaultCallManager<M: Machine> {
     limits: M::Limiter,
     /// Accumulator for events emitted in this call stack.
     events: EventsAccumulator,
+    code_upgrade_by_actor: HashMap<ActorID, i32>,
 }
 
 #[doc(hidden)]
@@ -159,6 +161,7 @@ where
             limits,
             events: Default::default(),
             state_access_tracker,
+            code_upgrade_by_actor: HashMap::new(),
         })))
     }
 
@@ -327,6 +330,10 @@ where
         self.nonce
     }
 
+    fn get_actor_upgrade_stack(&mut self) -> &mut HashMap<ActorID, i32> {
+        &mut self.code_upgrade_by_actor
+    }
+
     fn next_actor_address(&self) -> Address {
         // Base the next address on the address specified as the message origin. This lets us use,
         // e.g., an f2 address even if we can't look it up anywhere.

fvm/src/call_manager/mod.rs

@@ -7,6 +7,7 @@ use fvm_shared::econ::TokenAmount;
 use fvm_shared::error::ExitCode;
 use fvm_shared::upgrade::UpgradeInfo;
 use fvm_shared::{ActorID, MethodNum};
+use std::collections::HashMap;
 
 use crate::engine::Engine;
 use crate::gas::{Gas, GasCharge, GasTimer, GasTracker, PriceList};
@@ -119,6 +120,8 @@ pub trait CallManager: 'static {
         delegated_address: Option<Address>,
     ) -> Result<()>;
 
+    fn get_actor_upgrade_stack(&mut self) -> &mut HashMap<ActorID, i32>;
+
     /// Resolve an address into an actor ID, charging gas as appropriate.
     fn resolve_address(&self, address: &Address) -> Result<Option<ActorID>>;
 

fvm/src/kernel/default.rs

@@ -873,6 +873,14 @@ where
             .create_actor(code_id, actor_id, delegated_address)
     }
 
+    fn actor_on_upgrade_stack(&mut self) -> bool {
+        self.call_manager
+            .get_actor_upgrade_stack()
+            .get(&self.actor_id)
+            .map(|count| *count > 0)
+            .unwrap_or(false)
+    }
+
     fn upgrade_actor<K: Kernel>(
         &mut self,
         new_code_cid: Cid,
@@ -896,6 +904,12 @@ where
             return Err(syscall_error!(LimitExceeded; "cannot store return block").into());
         }
 
+        self.call_manager
+            .get_actor_upgrade_stack()
+            .entry(self.actor_id)
+            .and_modify(|count| *count += 1)
+            .or_insert(1);
+
         let result = self.call_manager.with_transaction(|cm| {
             let origin = cm.origin();
 

fvm/src/kernel/mod.rs

@@ -215,6 +215,8 @@ pub trait ActorOps {
         params_id: BlockId,
     ) -> Result<SendResult>;
 
+    fn actor_on_upgrade_stack(&mut self) -> bool;
+
     /// Installs actor code pointed by cid
     #[cfg(feature = "m2-native")]
     fn install_actor(&mut self, code_cid: Cid) -> Result<()>;

fvm/src/syscalls/actor.rs

@@ -121,6 +121,14 @@ pub fn upgrade_actor<K: Kernel>(
         Err(err) => return err.into(),
     };
 
+    // Check if this actor is already being upgraded.
+    if context.kernel.actor_on_upgrade_stack() {
+        return ControlFlow::Error(syscall_error!(
+            Forbidden;
+            "re-entrant upgrade detected"
+        ));
+    };
+
     match context.kernel.upgrade_actor::<K>(cid, params_id) {
         Ok(SendResult {
             block_id,

fvm/tests/dummy.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0, MIT
 use std::borrow::Borrow;
 use std::cell::RefCell;
+use std::collections::HashMap;
 use std::rc::Rc;
 
 use anyhow::Context;
@@ -358,6 +359,10 @@ impl CallManager for DummyCallManager {
         todo!()
     }
 
+    fn get_actor_upgrade_stack(&mut self) -> &mut HashMap<ActorID, i32> {
+        todo!()
+    }
+
     fn invocation_count(&self) -> u64 {
         todo!()
     }

testing/conformance/src/vm.rs

@@ -302,6 +302,10 @@ where
     fn upgrade_actor<KK>(&mut self, new_code_cid: Cid, params_id: BlockId) -> Result<SendResult> {
         self.0.upgrade_actor::<Self>(new_code_cid, params_id)
     }
+
+    fn actor_on_upgrade_stack(&mut self) -> bool {
+        self.0.actor_on_upgrade_stack()
+    }
 }
 
 impl<M, C, K> IpldBlockOps for TestKernel<K>

testing/integration/tests/main.rs

@@ -1188,9 +1188,6 @@ fn upgrade_actor_test() {
             .execute_message(message, ApplyKind::Explicit, 100)
             .unwrap();
 
-        let val: i64 = res.msg_receipt.return_data.deserialize().unwrap();
-        assert_eq!(val, 444);
-
         assert!(
             res.msg_receipt.exit_code.is_success(),
             "{:?}",

testing/test_actors/actors/fil-upgrade-actor/src/actor.rs

@@ -4,6 +4,7 @@ use fvm_ipld_encoding::ipld_block::IpldBlock;
 use fvm_ipld_encoding::{to_vec, CBOR};
 use fvm_sdk as sdk;
 use fvm_shared::address::Address;
+use fvm_shared::error::ErrorNumber;
 use fvm_shared::upgrade::UpgradeInfo;
 use serde_tuple::*;
 #[derive(Serialize_tuple, Deserialize_tuple, PartialEq, Eq, Clone, Debug)]
@@ -48,8 +49,9 @@ pub fn upgrade(params_id: u32, upgrade_info_id: u32) -> u32 {
             sdk::debug::log("[upgrade] params:3, calling upgrade within an upgrade".to_string());
             let new_code_cid = sdk::actor::get_actor_code_cid(&Address::new_id(10000)).unwrap();
             let params = IpldBlock::serialize_cbor(&SomeStruct { value: 4 }).unwrap();
-            let _ = sdk::actor::upgrade_actor(new_code_cid, params);
-            unreachable!("we should never return from a successful upgrade");
+            let res = sdk::actor::upgrade_actor(new_code_cid, params);
+            assert_eq!(res, Err(ErrorNumber::Forbidden));
+            0
         }
         4 => {
             let block_id = sdk::ipld::put_block(CBOR, &to_vec(&444).unwrap()).unwrap();
@@ -88,7 +90,7 @@ pub fn invoke(_: u32) -> u32 {
                 "invalid exit code returned from upgrade_actor"
             );
         }
-        // test recursive update
+        // test that calling recursive update on the same actor fails
         3 => {
             let new_code_cid = sdk::actor::get_actor_code_cid(&Address::new_id(10000)).unwrap();
             let params = IpldBlock::serialize_cbor(&SomeStruct { value: 3 }).unwrap();