tokamak-network / ton-staking-v2

11 stars 3 forks source link

[internal test - Contract Bugs]: Permissions need to be re-granted #52

Closed DevUreak closed 1 month ago

DevUreak commented 1 month ago

Describe the bug

Describe the bug and its behavior! You can reference the problematic code using links (internal-test-(2024.10) branch)

In a specific rollupConfig we can stop the seigniorage. The permission for this seems to be higher than the "Registrant"ROLE.

The code below is assumed to be the highest permission in the "L1BridgeRegistryV1_1" contract.

   function rejectCandidateAddOn(
        address rollupConfig
    )  external onlySeigniorageCommittee() {
        _nonRejected(rollupConfig);

        require (rollupInfo[rollupConfig].rollupType != 0, "NonRegistered");

        rollupInfo[rollupConfig].rejectedSeigs = true;
        rollupInfo[rollupConfig].rejectedL2Deposit = true;

        ILayer2Manager(layer2Manager).pauseCandidateAddOn(rollupConfig);
        emit RejectedCandidateAddOn(rollupConfig);
    }

However, A wallet granted the Registrant Role can restore a rejected seigniorage by calling changeType().

It seems like we need to block access to specific addresses or change the logic for changing changeType() permissions.

    function changeType(address rollupConfig, uint8 _type, address _l2TON, string calldata _name)  external  onlyRegistrant {

        ROLLUP_INFO memory info = rollupInfo[rollupConfig];
        if (info.rollupType == 0) revert ChangeError(1);
        if (info.rollupType == _type) revert ChangeError(2);
        if (_l2TON == address(0)) revert ChangeError(3);
        if (bytes(_name).length == 0) revert ChangeError(4);

        _resetRollupConfig(rollupConfig) ;
        _registerRollupConfig(rollupConfig, _type, _l2TON, _name);

        emit ChangedType(rollupConfig, _type, _l2TON, _name);
    }

Impact

No response

Exploit Scenario

No response

Recommendation

layer2/L1BridgeRegistryV1_1.sol

Zena-park commented 1 month ago

_resetRollupConfig function has a condition,

https://github.com/tokamak-network/ton-staking-v2/blob/37bac22659d03ac9e50455da3412ccf3ab5a6810/contracts/layer2/L1BridgeRegistryV1_1.sol#L437

If the seigniorage payment is stopped (info.rejectedSeigs == true), the changeType function execution fails.

DevUreak commented 1 month ago

Ah, I will write more carefully