fix: M-01 Space Deployment vulnerable to front-running (#581)

* add caller address to salt computation * type inference for caller_address * swap initialize_calldata and contract_address_salt argument order * rename contract_address_salt to salt_nonce * chore: update comment --------- Co-authored-by: Orland0x <[email protected]>
snapshot-labs · Oct 5, 2023 · c36adc3 · c36adc3
1 parent 65d0cc1
commit c36adc3
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 10 deletions.
diff --git a/starknet/src/factory/factory.cairo b/starknet/src/factory/factory.cairo
@@ -5,8 +5,8 @@ trait IFactory<TContractState> {
     fn deploy(
         ref self: TContractState,
         class_hash: ClassHash,
-        contract_address_salt: felt252,
-        initialize_calldata: Span<felt252>
+        initialize_calldata: Span<felt252>,
+        salt_nonce: felt252,
     ) -> SyscallResult<ContractAddress>;
 }
 
@@ -37,9 +37,15 @@ mod Factory {
         fn deploy(
             ref self: ContractState,
             class_hash: ClassHash,
-            contract_address_salt: felt252,
-            initialize_calldata: Span<felt252>
+            initialize_calldata: Span<felt252>,
+            salt_nonce: felt252,
         ) -> SyscallResult<ContractAddress> {
+            // We create the salt by hashing the user provided nonce and the caller address
+            // to avoid any frontrun attacks.
+            let caller_address = starknet::info::get_caller_address().into();
+            let salt_input = array![caller_address, salt_nonce];
+            let contract_address_salt = poseidon::poseidon_hash_span(salt_input.span());
+
             let (space_address, _) = syscalls::deploy_syscall(
                 class_hash, contract_address_salt, array![].span(), false
             )?;

diff --git a/starknet/src/tests/factory/factory.cairo b/starknet/src/tests/factory/factory.cairo
@@ -87,15 +87,15 @@ mod tests {
         let factory = IFactoryDispatcher { contract_address: factory_address };
 
         let space_class_hash: ClassHash = Space::TEST_CLASS_HASH.try_into().unwrap();
-        let contract_address_salt = 0;
+        let salt_nonce = 0;
 
         let config = setup();
         let constructor_calldata = config.get_initialize_calldata();
 
         let space_address = factory
-            .deploy(space_class_hash, contract_address_salt, constructor_calldata.span());
+            .deploy(space_class_hash, constructor_calldata.span(), salt_nonce);
         let space_address_2 = factory
-            .deploy(space_class_hash, contract_address_salt, constructor_calldata.span());
+            .deploy(space_class_hash, constructor_calldata.span(), salt_nonce);
     // TODO: this test should fail but doesn't fail currently because of how the test environment works
     }
 }
diff --git a/starknet/src/tests/setup/setup.cairo b/starknet/src/tests/setup/setup.cairo
@@ -119,7 +119,7 @@ mod setup {
 
     fn deploy(config: @Config) -> (IFactoryDispatcher, ISpaceDispatcher) {
         let space_class_hash: ClassHash = Space::TEST_CLASS_HASH.try_into().unwrap();
-        let contract_address_salt = 0;
+        let salt_nonce = 0;
 
         let factory_address =
             match deploy_syscall(
@@ -136,8 +136,7 @@ mod setup {
 
         let mut initializer_calldata = config.get_initialize_calldata();
         let space_address =
-            match factory
-                .deploy(space_class_hash, contract_address_salt, initializer_calldata.span()) {
+            match factory.deploy(space_class_hash, initializer_calldata.span(), salt_nonce) {
             Result::Ok(address) => address,
             Result::Err(e) => {
                 panic_with_felt252('deploy failed');