FIAT DAO veFDT contest - jonatascm's results

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L425-L428 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L485-L488 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L546 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L657 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L676

Vulnerability details

Vulnerability details

Some ERC20 tokens such as USDT don't return boolean values from transfer and transferFrom methods. The require checks will therefore revert causing the functions to be unusable.

Impact

This would prevent withdraw and createLock functions to be executed, for example.

Recommended Mitigation Steps

Use safeTransfer and safeTransferFrom methods of OpenZeppelin's SafeERC20 library instead of transfer and transferFrom:

// Case of transferFrom function:
- require(
-  token.transferFrom(msg.sender, address(this), _value),
-  "Transfer failed"
- );
+ token.safeTransferFrom(msg.sender, address(this), _value);

// Case of transfer function:
- require(token.transfer(msg.sender, value), "Transfer failed");
+ token.safeTransfer(msg.sender, value);

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L425-L428 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L485-L488 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L546 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L657 https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L676

Vulnerability details

Vulnerability details

If the token is set to a deflationary/fee-on-transfer token then its actual transfer amounts will not be accurately reflected in the protocol accounting given the lack of pre-transfer and post-transfer checks on asset transfers.

Impact

Some users may not receive their tokens as expected.

Recommended Mitigation Steps

Consider adding a check of balance pre-transfer and post-transfer, use this code as example:

uint256 _balance = token.balanceOf(address(this));
token.safeTransferFrom(msg.sender, address(this), _value);
uint256 balance_ = token.balanceOf(address(this));

//If deflationary/fee-on-transfer depositedAmount != _value
uint256 depositedAmount = balance_ - _balance;

Floating pragma is risky

All contracts in scope

Vulnerability details

The codebase uses floating pragma. All contracts should be compiled with same pragma version. Locking the pragma ensures that contracts do not accidentally get deployed using either an outdated buggy compiler version or a compiler version different from what the code has been tested with.

Recommended Mitigation Steps

Use the same compiler version for all contracts by setting a specific version e.g. 0.8.15  for these contracts.

Missing unblock functionality in Blocklist

Blocklist.sol

Vulnerability details

Blocklist contract should give support to unblock users.

Recommended Mitigation Steps

Add a function that unblock users.