Submission Details
Severity:
Against votes may accidentally make a proposal pass
Summary
Against votes may accidentally make a proposal pass.
Vulnerability Details
During a governance proposal's voting period, users can cast for or against votes, and the proposal's forVotes and againstVotes are updated accordingly.
if (support) {
proposalVote.forVotes += weight;
} else {
proposalVote.againstVotes += weight;
}
For the proposal to be passed, it requires that the current quorum is larger than the required quorum, and forVotes is larger than againstVotes, or the proposal is defeated.
// After voting period ends, check quorum and votes
ProposalVote storage proposalVote = _proposalVotes[proposalId];
uint256 currentQuorum = proposalVote.forVotes + proposalVote.againstVotes;
uint256 requiredQuorum = quorum();
// Check if quorum is met and votes are in favor
if (currentQuorum < requiredQuorum || proposalVote.forVotes <= proposalVote.againstVotes) {
return ProposalState.Defeated;
}
currentQuorum is the sum of forVotes and againstVotes, this is problematic as against votes may suddenly make a proposal which is not supposed to be passed pass.
Assuming requiredQuorum is 4200, Alice only has 3000 voting power, when she votes for a proposal, the proposal's currentQuorum is 3000, if no other users votes for, the proposal is not supposed to be passed. However, it is possible that Bob who has 2000 voting power votes againt the proposal (he even may vote before Alice), then currentQuorum becomes 5000, larger than requiredQuorum.
By the end of the voting period, the proposal turns out succeed, Bob's behavior accidentally against his own will.
Impact
Againt votes makes a proposal pass.
POC
Run forge test --mt testAudit_AgainstVotesMakeProposalPass.
// SPDX-License-Identifier: MIT
import {Test, console, stdError} from "forge-std/Test.sol";
import "@openzeppelin/contracts/utils/math/SafeCast.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import "../contracts/libraries/math/WadRayMath.sol";
import "../contracts/core/pools/LendingPool/LendingPool.sol";
import "../contracts/core/pools/StabilityPool/StabilityPool.sol";
import "../contracts/mocks/core/tokens/crvUSDToken.sol";
import "../contracts/core/tokens/RToken.sol";
import "../contracts/core/tokens/DebtToken.sol";
import "../contracts/core/tokens/DeToken.sol";
import "../contracts/core/tokens/RAACToken.sol";
import "../contracts/core/tokens/RAACNFT.sol";
import "../contracts/core/tokens/veRAACToken.sol";
import "../contracts/core/primitives/RAACHousePrices.sol";
import "../contracts/core/minters/RAACMinter/RAACMinter.sol";
import "../contracts/core/collectors/FeeCollector.sol";
import "../contracts/core/collectors/Treasury.sol";
import "../contracts/core/governance/proposals/Governance.sol";
import "../contracts/core/governance/proposals/TimelockController.sol";
contract Audit is Test {
using WadRayMath for uint256;
using SafeCast for uint256;
address owner = makeAddr("Owner");
address repairFund = makeAddr("RepairFund");
LendingPool lendingPool;
StabilityPool stabilityPool;
RAACHousePrices raacHousePrices;
crvUSDToken crvUSD;
RToken rToken;
DebtToken debtToken;
DEToken deToken;
RAACToken raacToken;
RAACNFT raacNft;
veRAACToken veRaacToken;
RAACMinter raacMinter;
FeeCollector feeCollector;
Treasury treasury;
Governance governance;
TimelockController timelockController;
function setUp() public {
vm.warp(1 days);
raacHousePrices = new RAACHousePrices(owner);
// Deploy tokens
raacToken = new RAACToken(owner, 100, 50);
veRaacToken = new veRAACToken(address(raacToken));
veRaacToken.transferOwnership(owner);
crvUSD = new crvUSDToken(owner);
rToken = new RToken("RToken", "RToken", owner, address(crvUSD));
debtToken = new DebtToken("DebtToken", "DT", owner);
raacNft = new RAACNFT(address(crvUSD), address(raacHousePrices), owner);
deToken = new DEToken("DEToken", "DEToken", owner, address(rToken));
// Deploy Treasury and FeeCollector
treasury = new Treasury(owner);
feeCollector = new FeeCollector(
address(raacToken),
address(veRaacToken),
address(treasury),
repairFund,
owner
);
// Deploy LendingPool
lendingPool = new LendingPool(
address(crvUSD),
address(rToken),
address(debtToken),
address(raacNft),
address(raacHousePrices),
0.1e27
);
lendingPool.transferOwnership(owner);
// Deploy stabilityPool Proxy
bytes memory data = abi.encodeWithSelector(
StabilityPool.initialize.selector,
address(rToken),
address(deToken),
address(raacToken),
address(owner),
address(crvUSD),
address(lendingPool)
);
address stabilityPoolProxy = address(
new TransparentUpgradeableProxy(
address(new StabilityPool(owner)),
owner,
data
)
);
stabilityPool = StabilityPool(stabilityPoolProxy);
// RAACMinter
raacMinter = new RAACMinter(
address(raacToken),
address(stabilityPool),
address(lendingPool),
owner
);
// Governance
address[] memory proposers;
address[] memory executors;
timelockController = new TimelockController(2 days, proposers, executors, owner);
governance = new Governance(address(veRaacToken), address(timelockController));
// Initialization
vm.startPrank(owner);
raacHousePrices.setOracle(owner);
rToken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
deToken.setStabilityPool(address(stabilityPool));
stabilityPool.setRAACMinter(address(raacMinter));
lendingPool.setStabilityPool(address(stabilityPool));
raacToken.setMinter(address(raacMinter));
raacToken.setFeeCollector(address(feeCollector));
raacToken.manageWhitelist(address(feeCollector), true);
raacToken.manageWhitelist(address(raacMinter), true);
raacToken.manageWhitelist(address(stabilityPool), true);
raacToken.manageWhitelist(address(veRaacToken), true);
timelockController.grantRole(keccak256("PROPOSER_ROLE"), address(governance));
timelockController.grantRole(keccak256("EXECUTOR_ROLE"), address(governance));
timelockController.grantRole(keccak256("CANCELLER_ROLE"), address(governance));
timelockController.grantRole(keccak256("EMERGENCY_ROLE"), address(governance));
vm.stopPrank();
vm.label(address(crvUSD), "crvUSD");
vm.label(address(rToken), "RToken");
vm.label(address(debtToken), "DebtToken");
vm.label(address(deToken), "DEToken");
vm.label(address(raacToken), "RAACToken");
vm.label(address(raacNft), "RAAC NFT");
vm.label(address(lendingPool), "LendingPool");
vm.label(address(stabilityPool), "StabilityPool");
vm.label(address(raacMinter), "RAACMinter");
vm.label(address(veRaacToken), "veRAAC");
vm.label(address(feeCollector), "FeeCollector");
vm.label(address(treasury), "Treasury");
vm.label(repairFund, "RepairFund");
}
function testAudit_AgainstVotesMakeProposalPass() public {
uint256 balance = 100_000e18;
address proposer = makeAddr("Proposer");
deal(address(raacToken), proposer, balance);
vm.startPrank(proposer);
raacToken.approve(address(veRaacToken), balance);
veRaacToken.lock(balance, 1460 days);
vm.stopPrank();
// Propose
address[] memory targets = new address[](1);
targets[0] = address(raacToken);
uint256[] memory values = new uint256[](1);
values[0] = 0;
bytes[] memory calldatas = new bytes[](1);
calldatas[0] = abi.encodeWithSignature("setBurnTaxRate(uint256)", 55);
string memory description = "Update RAACToken burnTaxRate to 55";
IGovernance.ProposalType proposalType = IGovernance.ProposalType.ParameterChange;
vm.prank(proposer);
uint256 proposalId = governance.propose(targets, values, calldatas, description, proposalType);
// Vote starts
vm.warp(block.timestamp + 1 days);
uint256 aliceBalance = 3_000e18;
address alice = makeAddr("Alice");
deal(address(raacToken), alice, aliceBalance);
vm.startPrank(alice);
raacToken.approve(address(veRaacToken), aliceBalance);
veRaacToken.lock(aliceBalance, 1460 days);
vm.stopPrank();
uint256 bobBalance = 2_000e18;
address bob = makeAddr("Bob");
deal(address(raacToken), bob, bobBalance);
vm.startPrank(bob);
raacToken.approve(address(veRaacToken), bobBalance);
veRaacToken.lock(bobBalance, 1460 days);
vm.stopPrank();
// quorum is 4200
uint256 quorum = governance.quorum();
assertEq(quorum, 4200e18);
// Alice votes for
vm.prank(alice);
governance.castVote(proposalId, true);
{
// quorum is not met, it is not supposed to pass
(uint256 forVotes, uint256 againstVotes) = governance.getVotes(proposalId);
assertLt(forVotes + againstVotes, quorum);
}
// Bob votes against
vm.prank(bob);
governance.castVote(proposalId, false);
{
// quorum is met, and forVotes is larger than againstVotes, the proposal will pass
// however, it's not Bob's intention to make the propsal pass...
(uint256 forVotes, uint256 againstVotes) = governance.getVotes(proposalId);
assertGt(forVotes + againstVotes, quorum);
assertGt(forVotes, againstVotes);
}
// Vote ends
vm.warp(block.timestamp + 7 days);
// Proposal is passed
IGovernance.ProposalState state = governance.state(proposalId);
assertEq(uint8(state), uint8(IGovernance.ProposalState.Succeeded));
}
}
Tools Used
Manual Review
Recommendations
When calculate currentQuorum of a proposal, do not count in against votes.
// After voting period ends, check quorum and votes
ProposalVote storage proposalVote = _proposalVotes[proposalId];
- uint256 currentQuorum = proposalVote.forVotes + proposalVote.againstVotes;
+ uint256 currentQuorum = proposalVote.forVotes ;
uint256 requiredQuorum = quorum();
// Check if quorum is met and votes are in favor
if (currentQuorum < requiredQuorum || proposalVote.forVotes <= proposalVote.againstVotes) {
return ProposalState.Defeated;
}
Updates
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420 USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.