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

0 stars 0 forks source link

Anyone can call admin functions in RollupAdminLogic.sol #384

Open c4-bot-9 opened 4 months ago

c4-bot-9 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/rollup/RollupAdminLogic.sol#L18-L330

Vulnerability details

Summary

Lack of access control in RollupAdminLogic.sol.

Proof of Concept

It is assumed that only admin can call functions from RollupAdminLogic.sol. But anyone can call this functions, as there is no restriction modifiers in all functions, here is one example:

function setValidator(address[] calldata _validator, bool[] calldata _val) external override {
        require(_validator.length > 0, "EMPTY_ARRAY");
        require(_validator.length == _val.length, "WRONG_LENGTH");

        for (uint256 i = 0; i < _validator.length; i++) {
            isValidator[_validator[i]] = _val[i];
        }
        emit OwnerFunctionCalled(6);
    }

The only modifiers is in functions that interact with bridge:

function setOutbox(address outbox, bool enabled) external onlyRollupOrOwner {
//…
modifier onlyRollupOrOwner() {
        if (msg.sender != address(rollup)) {
            address rollupOwner = rollup.owner();
            if (msg.sender != rollupOwner) {
                revert NotRollupOrOwner(msg.sender, address(rollup), rollupOwner);
            }
        }
        _;
    }

As we can see, it only checks if call was made from RollupAdminLogic, but it does not check if admin calls this function in RollupAdminLogic.

Impact

Anyone can call admin functions.

Tools Used

Manual review.

Recommended Mitigation Steps

Add onlyOwner modifier in all functions in RollupAdminLogic.sol.

Assessed type

Access Control