Olympus DAO contest - bin2chen's results

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L164 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L216 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Governance.sol#L268

Vulnerability details

Impact

In the initial deployment, (or all burn) VOTES.totalSupply=0, you can submit any proposal, and become activeProposal without votes if EXECUTION_TIMELOCK days later (3 days), still VOTES.totalSupply=0, then the proposal can be executed without votes

Proof of Concept

1.submitProposal() when totalSupply()=0 , you can pass without votes

    function submitProposal(
        Instruction[] calldata instructions_,
        bytes32 title_,
        string memory proposalURI_
    ) external {
        //****** when totalSupply()=0 , you can pass without votes ***//
        if (VOTES.balanceOf(msg.sender) * 10000 < VOTES.totalSupply() * SUBMISSION_REQUIREMENT)
            revert NotEnoughVotesToPropose();

2.activateProposal() when totalSupply()=0 , you can pass without votes

    function activateProposal(uint256 proposalId_) external {
        ...

        //****** when totalSupply()=0 , you can pass without votes***//
        if (
            (totalEndorsementsForProposal[proposalId_] * 100) <
            VOTES.totalSupply() * ENDORSEMENT_THRESHOLD
        ) {
            revert NotEnoughEndorsementsToActivateProposal();
        }

3.executeProposal() After three days, if totalSupply is still 0, you can execute it directly

    function executeProposal() external {
        //****** After three days, if totalSupply is still 0, you can execute it directly ***//
        if (netVotes * 100 < VOTES.totalSupply() * EXECUTION_THRESHOLD) {
            revert NotEnoughVotesToExecute();
        }

        if (block.timestamp < activeProposal.activationTimestamp + EXECUTION_TIMELOCK) {
            revert ExecutionTimelockStillActive();
        }

        ...

Tools Used

Recommended Mitigation Steps

Determine if the votes is satisfied, using less than or equal to

    function submitProposal(
        Instruction[] calldata instructions_,
        bytes32 title_,
        string memory proposalURI_
    ) external {

 ---    if (VOTES.balanceOf(msg.sender) * 10000 < VOTES.totalSupply() * SUBMISSION_REQUIREMENT)
 +++    if (VOTES.balanceOf(msg.sender) * 10000 <= VOTES.totalSupply() * SUBMISSION_REQUIREMENT)        
            revert NotEnoughVotesToPropose();

    }


    function activateProposal(uint256 proposalId_) external {
        ...

        if (
---         (totalEndorsementsForProposal[proposalId_] * 100) <
+++         (totalEndorsementsForProposal[proposalId_] * 100) <=
            VOTES.totalSupply() * ENDORSEMENT_THRESHOLD
        ) {
            revert NotEnoughEndorsementsToActivateProposal();
        }

    function executeProposal() external {

---     if (netVotes * 100 < VOTES.totalSupply() * EXECUTION_THRESHOLD) {
+++     if (netVotes * 100 <= VOTES.totalSupply() * EXECUTION_THRESHOLD) {
            revert NotEnoughVotesToExecute();
        }

        ...

    }

1: OlympusRange.sol constructor() need check params

    constructor(
        Kernel kernel_,
        ERC20[2] memory tokens_,
        uint256[3] memory rangeParams_ // [thresholdFactor, cushionSpread, wallSpread]
    ) Module(kernel_) {

+++    if (rangeParams_[0] > 10000 || rangeParams_[0] < 100) revert RANGE_InvalidParams();

+++    if (
+++         rangeParams_[2] > 10000 ||
+++         rangeParams_[2] < 100 ||
+++         rangeParams_[1] > 10000 ||
+++         rangeParams_[1] < 100 ||
+++         rangeParams_[1] > rangeParams_[2]
+++    ) revert RANGE_InvalidParams();        

2: OlympusHeart.sol constructor() add emit RewardUpdated，like setRewardTokenAndAmount()

    constructor(
        Kernel kernel_,
        IOperator operator_,
        ERC20 rewardToken_,
        uint256 reward_
    ) Policy(kernel_) {

        ...

        rewardToken = rewardToken_;
        reward = reward_;
+++     emit RewardUpdated(token_, reward_);
    }

3: Kernel.sol executeAction() add check target_！= address(0)

   function executeAction(Actions action_, address target_) external onlyExecutor {

        ...

        } else if (action_ == Actions.ChangeExecutor) {
+++         require(target!=address(0));
            executor = target_; 
        } else if (action_ == Actions.ChangeAdmin) {
+++         require(target!=address(0));        
            admin = target_; 
        } else if (action_ == Actions.MigrateKernel) {
        ...

4: OlympusVotes mintTo() burnFrom() add check wallet_ != address(0)

    function mintTo(address wallet_, uint256 amount_) external permissioned {    
+++     require(wallet_!=address(0));  
        _mint(wallet_, amount_);
    }

    function burnFrom(address wallet_, uint256 amount_) external permissioned {
+++     require(wallet_!=address(0));      
        _burn(wallet_, amount_);
    }