As a best practice, the return values for transfer/transferFrom function should be checked to ensure that the call succeeded or failed.
The implementation of functions FjordAuction does not check the return values for transfer/transferFrom functions.
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);
}
* @notice Allows users to withdraw part or all of their bids before the auction ends.
* @param amount The amount of FjordPoints to withdraw.
*/
function unbid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoBidsToWithdraw();
}
if (amount > userBids) {
revert InvalidUnbidAmount();
}
bids[msg.sender] = bids[msg.sender].sub(amount);
totalBids = totalBids.sub(amount);
--> fjordPoints.transfer(msg.sender, amount);
emit BidWithdrawn(msg.sender, amount);
}
* @notice Ends the auction and calculates claimable tokens for each bidder based on their bid proportion.
*/
function auctionEnd() external {
if (block.timestamp < auctionEndTime) {
revert AuctionNotYetEnded();
}
if (ended) {
revert AuctionEndAlreadyCalled();
}
ended = true;
emit AuctionEnded(totalBids, totalTokens);
if (totalBids == 0) {
--> auctionToken.transfer(owner, totalTokens);
return;
}
multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}
* @notice Allows users to claim their tokens after the auction has ended.
*/
function claimTokens() external {
if (!ended) {
revert AuctionNotYetEnded();
}
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoTokensToClaim();
}
uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
bids[msg.sender] = 0;
--> auctionToken.transfer(msg.sender, claimable);
emit TokensClaimed(msg.sender, claimable);
}
There is a potential that the internal account will break if the transfer() or transferFrom() actually fails, but since the return value is being ignored, the calling contract assumes that it will always succeed. This will break the internal accounting for the calling contract.
Always check the return values for the transfer() and transferFrom() functions. Some ERC tokens does not follow the standard, hence it is recommended to use safeTransfer functions instead.
