Anyone can cancel any finalising stage proposal by simply using newProposalID equal to an existing (oldProposalID) finalising proposal of their choice. All checks (finalising, hasMinority, isEqual) will pass for the call cancelProposal(oldProposalID, oldProposalID) because the oldProposalID has minority support (L104 check) as it’s already finalising and L105 check will also pass.
There is no need to create and vote on a new proposal with newProposalID that has minority support and has the same type string as the oldProposalID, to allow the canceling of the proposal with oldProposalID.
Handle
0xRajeev
Vulnerability details
Impact
Anyone can cancel any finalising stage proposal by simply using newProposalID equal to an existing (oldProposalID) finalising proposal of their choice. All checks (finalising, hasMinority, isEqual) will pass for the call cancelProposal(oldProposalID, oldProposalID) because the oldProposalID has minority support (L104 check) as it’s already finalising and L105 check will also pass.
There is no need to create and vote on a new proposal with newProposalID that has minority support and has the same type string as the oldProposalID, to allow the canceling of the proposal with oldProposalID.
Proof of Concept
https://github.com/code-423n4/2021-04-vader/blob/3041f20c920821b89d01f652867d5207d18c8703/vader-protocol/contracts/DAO.sol#L102-L108
Tools Used
Manual Analysis
Recommended Mitigation Steps
Perform input validation on parameters to check that newProposalID != oldProposalID in cancelProposal().