code-423n4 / 2023-07-arcade-findings

2 stars 1 forks source link

Changes of approved vaults during voting period #256

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-arcade/blob/f8ac4e7c4fdea559b73d9dd5606f618d4e6c73cd/contracts/external/council/CoreVoting.sol#L234

Vulnerability details

Impact

The malicious proposals can get approved and be executed immediately.

Proof of Concept

The vote counting process does not take into account potential changes of the list of associated vaults in the middle of the voting period. The vote tally that has already been counted will never be recounted even if the vault list is changed in the middle.

    for (uint256 i = 0; i < votingVaults.length; i++) {
        // ensure there are no voting vault duplicates
        for (uint256 j = i + 1; j < votingVaults.length; j++) {
            require(votingVaults[i] != votingVaults[j], "duplicate vault");
        }
        require(approvedVaults[votingVaults[i]], "unverified vault");
        votingPower += uint128(
            IVotingVault(votingVaults[i]).queryVotePower(
                msg.sender,
                proposals[proposalId].created,
                extraVaultData[i]
            )
        );
    }

Suppose a voting vault has been compromised and it grants a large voting power to adversaries, and they voted for malicious proposals using the compromised voting power. Suppose the governance realizes the situation during the voting period for the malicious proposals, and thus removes the compromised vault from the approved list. However, the adversaries’ votes that have already been made with the compromised vault will never be revoked, even if the vault is removed from the list. The only way to prevent the approval of the proposals is that sufficiently many benign users vote against them. However, suppose that the remaining voting period is too short for that, and it ends before that happens. Then, the malicious proposals are approved and can be executed immediately.

Tools Used

Manually

Recommended Mitigation Steps

Implement a way to handle properly the change of approved vaults

Assessed type

Other

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Invalid