code-423n4 / 2021-04-vader-findings

1 stars 0 forks source link

You can vote for proposal already completed #268

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

s1m0

Vulnerability details

Impact

The mapPID_finalised is set on completeProposal() but voteProposal() doesn't check it. A malicious user with enough capital and/or together with other people could execute a proposal in his favor (e.g. a grant) infinitely.

Proof of Concept

https://github.com/code-423n4/2021-04-vader/blob/main/vader-protocol/contracts/DAO.sol#L131 1 newGrantProposal() 2 voteProposal() until enough votes 3 finaliseProposal() -> grantFunds() -> completeProposal() 4 repeat

Tools Used

Manual analysis.

Recommended Mitigation Steps

In voteProposal() require(!mapPID_finalised[proposalID], "Proposal already executed")

strictly-scarce commented 3 years ago

If they have this weighting in the DAO, then they can pass another proposal of same nature.

GRANT funding is rate-limited for this reason.

strictly-scarce commented 3 years ago

However seems wise to add this check to force a new proposal

strictly-scarce commented 3 years ago

https://github.com/code-423n4/2021-04-vader-findings/issues/282

0xBrian commented 3 years ago

Unused mapPID_finalised addressed https://github.com/vetherasset/vaderprotocol-contracts/commit/6f961e6cd159e905ef53a5a067f956d21f8a46ee

dmvt commented 3 years ago

duplicate of #229