FIAT DAO veFDT contest - cryptphi's results

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L555-L592

Vulnerability details

Impact

In the delegation() call, there is no check if the caller's lock has reached expiration. With this a user whose lock has expired can delegate to other running locks so long the amount locked is not withdrawn.

Above can be used to influence voting power

Tools Used

Manual review

Recommended Mitigation Steps

additional require check to ensure the delegator's lock is not expired.

1. missing zero address check The following are missing zero address checks which may lead to contract redeployment, reverts, loss of ownership to contract and/or loss of funds.

**Occurrences:

• Blocklist.constructor() - _manager and _ve params are missing zero address check which would lead to contract redeployment.
• VotingEscrow.constructor() - params are missing zero address check which would lead to contract redeployment.
• VotingEscrow.transferOwnership() - transferring ownership to zero address would make address(0) and ability to make admin calls lost permanently.
• VotingEscrow.updateBlocklist() - setting blockslist address to address(0) would make many functions in contract revert.
• VotingEscrow.updatePenaltyRecipient() - setting penaltyRecipient to address(0) may lead to loss of funds when collecting Penalty in collectPenalty()

2. Blocklist contract should inherit from IBlocklist The Blocklist contract is not inheriting IBlocklist.

3. Floating Pragma Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

**Occurrences:

• IVotingEscrow.sol
• VotingEscrow.sol
• IBlocklist.sol
• Blocklist.sol
• IERC20.sol

4. Developer comment in Blocklist.block() should be @dev only callable by manager. instead of @dev only callable by owner.