Initialize function can be called by any external address

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/main/code/contracts/ethereum/contracts/state-transition/StateTransitionManager.sol#L79 https://github.com/code-423n4/2024-03-zksync/blob/main/code/contracts/ethereum/contracts/bridge/L1SharedBridge.sol#L104

Vulnerability details

Impact

No form of access control will lead to the call of the initialize function by any external address

Proof of Concept

Consider the initialize function: function initialize(StateTransitionManagerInitializeData calldata _initializeData) external reentrancyGuardInitializer { require(_initializeData.governor != address(0), "StateTransition: governor zero"); _transferOwnership(_initializeData.governor);

// Initialization logic...

}

The absence of an explicit access control modifier is a serious security issue as it can permit an attacker to reinitialize the contract or tamper with its state

Tools Used

Manual Review

Recommended Mitigation Steps

The OnlyOwner modifier or an appropriate access control mechanism (say a msg.sender require statement) should be included in the function

Assessed type

Access Control

[c4-sponsor]

saxenism (sponsor) disputed

[saxenism]

Proxy deployment does not need access control. We are not calling the implementation directly.

Agreed with the Invalid tag. From the docs: In the first release creating new chains as well as SharedBridge initialization for new chains will not be permissionless

[razzorsec]

Adding to @saxenism comment, the implementations either disable initializers at the construction with _disableInitializers or reentrancyGuardInitializer (which behaves in the same way as it checks that the lockSlotOldValue must be 0 for any subsequent calls)

[alex-ppg]

The Warden specifies that contracts may be re-initialized due to the absence of access control. This is incorrect, as both contracts utilize a special reentrancyGuardInitializer that effectively prevents re-initialization.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid