Fjord Token Staking

Summary

This report highlights the issue of ignored return values from transfer and transferFrom functions in multiple contract functions of the FjordAuction and AuctionFactory contracts. Such practices can lead to unverifiable fund transfers, causing potential security and reliability issues.

Vulnerability Details

The following functions in the FjordAuction and AuctionFactory contracts ignore the return values from the transfer and transferFrom functions of ERC20 tokens:

1. FjordAuction.bid(uint256) (line 145-155):

   • Ignored return value from fjordPoints.transferFrom(msg.sender, address(this), amount) at line 153.

2. FjordAuction.unbid(uint256) (line 163-180):

   • Ignored return value from fjordPoints.transfer(msg.sender, amount) at line 178.

3. FjordAuction.auctionEnd() (line 186-207):

   • Ignored return value from auctionToken.transfer(owner, totalTokens) at line 198.

4. FjordAuction.claimTokens() (line 212-227):

   • Ignored return value from auctionToken.transfer(msg.sender, claimable) at line 225.

5. AuctionFactory.createAuction(address, uint256, uint256, bytes32) (line 54-72):

   • Ignored return value from IERC20(auctionToken).transferFrom(msg.sender, auctionAddress, totalTokens) at line 69.

Impact

Ignoring the return value of the transfer and transferFrom functions can lead to several serious issues:

1. Unverifiable Fund Transfers: The ERC20 standard returns a boolean indicating the success or failure of the transfer. Ignoring these return values means failures (such as insufficient allowance or balance) can go unnoticed. This can lead to scenarios where the contract logic assumes tokens have been transferred when they haven't.

2. Potential Reentrancy: While current implementations of the transfer and transferFrom functions do not pose reentrancy risks, relying on their return value for fund transfer validation is vital in safeguarding against potential future changes in ERC20 token implementations.

3. Loss of Funds: If failed transactions are not caught, users can lose tokens, leading to trust issues and possibly legal implications.

4. Exploit Scenario: An attacker can exploit this vulnerability by making calls to functions like deposit without actually transferring tokens if the token fails to transfer and returns false. This can allow attackers to manipulate their balances without contributing actual funds.

Tools Used

• Manual code review

Recommendations

• To address these issues, implement the following recommendations:

  1. Check Return Values: Ensure that the return values from transfer and transferFrom functions are checked and handled appropriately. For example:

     require(fjordPoints.transferFrom(msg.sender, address(this), amount), "Transfer failed");

     ​

     Kopier kode

  2. Use SafeERC20 Library: Consider using OpenZeppelin's SafeERC20 library, which internally handles these checks:

     import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

     ​

     using SafeERC20 for IERC20;

     ​

     fjordPoints.safeTransferFrom(msg.sender, address(this), amount);