code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/features/Blocklist.sol#L23-L28 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L170-L183 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L555-L592

Vulnerability details

Impact

One can avoid being blocked.

Proof of Concept

1) block(target) is broadcasted by the owner to mempool 2) target exits using quitLock(), pays penalty - this transaction is put before the (1) - frontrun 3) (2) happens, then (2) happens 4) target enter with the fresh account / smart-contract

Tools Used

Hardhat

Recommended Mitigation Steps

quitLock() should have an action in a few blocks after the initial call, otherwise fail the quitLock()

lacoop6tu commented 2 years ago

Being able to quitLock anytime is part of the mechanism, if a contract is blocked is because we need to limit its ability to interact, if the contract quitLocks, it will pay a fee, and if it re-enters with the amount left, it might be forced to quitLock again for not being blocked, and repeat.. until the contract has no token left

elnilz commented 2 years ago

as @lacoop6tu mentioned its not a bug and defn not Med Risk as protocol operates as intended even if blocked user frontruns block tx by quitLock-ing. However, since technically blocked users can quitLock by frontrunning we may consider allowing blocked users to use quitLock in the first place. the only reason we did not was that some additional checks need be in place around blocked, quitted locks. so if anything, this is a UX issue and should be labeled QA severity

gititGoro commented 2 years ago

The sponsor has indicated they want to handle this with incentives (quitLock penalty). Since they've marked as acknowledged, I'm downgrading the severity to QA.

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