zkSync Era - twcctop's results

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/70b820d66fa4d30737e905a7fc807dc6b6c95b3b/code/contracts/ethereum/contracts/governance/Governance.sol#L256-L259

Vulnerability details

Impact

Malicious securityCouncil may dos updateSecurityCouncil .New security council is not able to be set,because old security council can always cancel the operation.

Proof of Concept

 function updateSecurityCouncil(address _newSecurityCouncil) external onlySelf {
        emit ChangeSecurityCouncil(securityCouncil, _newSecurityCouncil);
        securityCouncil = _newSecurityCouncil;
    }

In function updateSecurityCouncil , the modifier is onlySelf. It means that this function can only be called by execute function. A new operation is needed to scheduled.The issue is if a old SecurityCouncil notice that he is going to be replaced, he can cancel the operation immediately. So the securitycouncil will never change if he want. Make the council change logic useless.

 function cancel(bytes32 _id) external onlyOwnerOrSecurityCouncil {
        require(isOperationPending(_id), "Operation must be pending");
        delete timestamps[_id];
        emit OperationCancelled(_id);
    }

Tools Used

manual

Recommended Mitigation Steps

Consider limit the old security council authority in updateSecurityCouncil

Assessed type

Access Control