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

Unchecked Return Value in transfer and transferFrom Functions

bbash

Summary

The FjordAuction smart contract fails to check the return values of the transfer and transferFrom functions from the ERC20 standard. These functions return a boolean indicating the success or failure of the transfer. Failing to check these return values can lead to silent failures, where the contract behaves as though a transfer was successful even if it wasn’t, potentially leading to significant security risks, such as loss of tokens, incorrect state updates, and inconsistencies.

Vulnerability Details

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L143-L153

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
​
bids[msg.sender] = bids[msg.sender].add(amount);
totalBids = totalBids.add(amount);
​
fjordPoints.transferFrom(msg.sender, address(this), amount);
emit BidAdded(msg.sender, amount);
}

In this code, if the fjordPoints.transferFrom call fails (e.g., due to insufficient allowance or balance), the function will still proceed, recording the bid in the bids mapping and emitting the BidAdded event, even though the transfer did not actually occur.

A similar Issue is observed in the unbid and claimTokens functions.

Impact

If a transfer fails, the contract will not detect it, leading to potential loss of funds, misallocation of auction tokens, and incorrect bid tracking.

Tools Used

Manual review

Recommendations

To mitigate this vulnerability, you should always check the return values of transfer and transferFrom calls and revert the transaction if the transfer fails.

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
​
bids[msg.sender] = bids[msg.sender].add(amount);
totalBids = totalBids.add(amount);
​
bool success = fjordPoints.transferFrom(msg.sender, address(this), amount);
require(success, "Transfer failed"); //add this
emit BidAdded(msg.sender, amount);
}

Similarly, update the unbid and claimTokens functions.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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