Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Possible loss of funds, unsafe `transfer` functions can fail

0x1422

Summary

The transfer and transferFrom() function from ERC20 standard emits booleanas return value which shows a transaction is successful or not. But In the FjordAuction.solcontract, in many instances the check is missing which may break the protocol's functionality.

Vulnerability Details

The protocol's contracts are expected to used USDT, USDC and DAI. The contracts will be deployed on Any EVM compatible chain which also includes Ethereum mainnet itself. This issue is specifically for tokens like USDT and similar tokens etc on Ethereum mainnet.

The transfer() and transferFrom() funciton is used at following instances:

function claimTokens() external {
...
​
@> auctionToken.transfer(msg.sender, claimable);
}
​
function auctionEnd() external {
...
if (totalBids == 0) {
@> auctionToken.transfer(owner, totalTokens);
return;
}
}
function unbid(uint256 amount) external {
...
@> fjordPoints.transfer(msg.sender, amount);
}
function bid(uint256 amount) external {
@> fjordPoints.transferFrom(msg.sender, address(this), amount);
​
}

The issue here is with the use of unsafe transfer() function. The ERC20.transfer() function return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead.

Some tokens like USDT don't correctly implement the EIP20 standard and their transfer() function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert.

Impact

Tokens that don't actually perform the transfer and return false are still counted as a correct transfer and tokens that don't correctly implement the latest EIP20 spec will be unusable in the protocol as they revert the transaction because of the missing return value. This will lead to loss of user funds.

Tools Used

Manual Review

Recommendations

Use OpenZeppelin's SafeERC20 versions with the safeTransfer() function instead of transfer().

For example, consider below changes in FjordAuction.sol:

+ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract FjordAuction {
​
+ using SafeERC20 for IERC20;
​
​
function claimTokens() external {
...
​
- auctionToken.transfer(msg.sender, claimable);
+ auctionToken.safeTransfer(msg.sender, claimable);
}
​
function auctionEnd() external {
...
if (totalBids == 0) {
- auctionToken.transfer(owner, totalTokens);
+ auctionToken.safeTransfer(owner, totalTokens);
return;
}
}
​
function unbid(uint256 amount) external {
...
- fjordPoints.transfer(msg.sender, amount);
+ fjordPoints.safeTransfer(msg.sender, amount);
}
​
function bid(uint256 amount) external {
- fjordPoints.transferFrom(msg.sender, address(this), amount);
+ fjordPoints.safeTransferFrom(msg.sender, address(this), amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.