code-423n4 / 2024-05-arbitrum-foundation-validation

0 stars 0 forks source link

Protocol's deployment of new contracts is unsafe #351

Open c4-bot-6 opened 4 months ago

c4-bot-6 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/BOLDUpgradeAction.sol#L516-L519

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/BOLDUpgradeAction.sol#L516-L519


        RollupProxy rollup = new RollupProxy{salt: rollupSalt}();
        require(address(rollup) == expectedRollupAddress, "UNEXPCTED_ROLLUP_ADDR");

And some instances even have the salt as 0 like here: https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/assertionStakingPool/AssertionStakingPoolCreator.sol#L15-L22

function createPool(address _rollup, bytes32 _assertionHash)
  external
  returns (IAssertionStakingPool)
{
  AssertionStakingPool assertionPool = new AssertionStakingPool{ salt: 0 }(
    _rollup,
    _assertionHash
  );
  emit NewAssertionPoolCreated(_rollup, _assertionHash, address(assertionPool));
  return assertionPool;
}

Or https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/assertionStakingPool/EdgeStakingPoolCreator.sol#L19-L20

        EdgeStakingPool pool = new EdgeStakingPool{salt: 0}(challengeManager, edgeId);

And multiple other instances where the new() keyword is being used to deploy contracts, problem however is that this method makes it susceptible to the re-orging issues considering the contracts are to be deployed on Arbitrum.

Other instances of heavy usage of the new keyword can be pinpointed from this: https://github.com/search?q=repo%3Acode-423n4%2F2024-05-arbitrum-foundation+%3D+new+NOT+language%3AMarkdown+NOT+language%3ATypeScript+NOT+language%3AJavaScript&type=code.

Impact

Deployments are unsafe in some cases are unsafe, most especially in cases where the salt is 0.

Recommended Mitigation Steps

Consider passing in a valid salt value and having the deployment done in a safe way.

Assessed type

Context