search

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

2 stars 1 forks source link

QA Report #214

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. Use safeTransfer/safeTransferFrom consistently instead of transfer/transferFrom

It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

Instances:

contracts/VotingEscrow.sol:426:            token.transferFrom(msg.sender, address(this), _value),
contracts/VotingEscrow.sol:486:            token.transferFrom(msg.sender, address(this), _value),
contracts/VotingEscrow.sol:546:        require(token.transfer(msg.sender, value), "Transfer failed");
contracts/VotingEscrow.sol:657:        require(token.transfer(msg.sender, remainingAmount), "Transfer failed");
contracts/VotingEscrow.sol:676:        require(token.transfer(penaltyRecipient, amount), "Transfer failed");

Reference:

This similar medium-severity finding from Consensys Diligence Audit of Fei Protocol.

Recommended Mitigation Steps:

Consider using safeTransfer/safeTransferFrom or require() consistently.

2. USE OF FLOATING PRAGMA

Recommend using fixed solidity version

Instances

All contracts in scope contains floating pragma: https://github.com/code-423n4/2022-08-fiatdao#files-in-scope

contracts/VotingEscrow.sol:2:pragma solidity ^0.8.3;
contracts/interfaces/IERC20.sol:2:pragma solidity ^0.8.3;
contracts/interfaces/IVotingEscrow.sol:2:pragma solidity ^0.8.3;
contracts/interfaces/IBlocklist.sol:2:pragma solidity ^0.8.3;
contracts/features/Blocklist.sol:2:pragma solidity ^0.8.3;