code-423n4 / 2022-08-fiatdao-findings

2 stars 1 forks source link

VotingEscrow: Anyone can call the collectPenalty function #191

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L673-L678

Vulnerability details

Impact

In the VotingEscrow contract, anyone can call the collectPenalty function to send penalty tokens to the penaltyRecipient address. However, if the private key of the penaltyRecipient address is compromised, the attacker can immediately call the collectPenalty function to transfer the penalty tokens from the contract. In addition, if the penaltyRecipient is not set correctly (to address 0 or some other incorrect address), a malicious user can also call the collectPenalty function to destroy the penalty tokens in the contract

Proof of Concept

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L673-L678

Tools Used

None

Recommended Mitigation Steps

Consider allowing only the owner to call the collectPenalty function

lacoop6tu commented 2 years ago

Duplicate of #13

gititGoro commented 2 years ago

None of these vectors are clearly bugs but could easily be design decisions. Restricting access to something doesn't automatically improve a protocol.