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

Improper auctionToken flow in case of no bids

tpiliposian

Summary

The auction contract's logic unintentionally transfers auctionToken back to the factory contract when no bids are placed during an auction. This is because the auctionEnd() function transfers auctionToken to the auction contract's owner, which is the factory contract. However, there is no function in the factory contract to retrieve these tokens, which creates a locked token scenario.

Vulnerability Details

In the event that no bids are placed in the auction, the following code executes within the auctionEnd() function of the FjordAuction contract:

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);
​
// Burn the FjordPoints held by the contract
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}

Here, the auctionToken is transferred back to the owner of the auction, which is the factory contract (AuctionFactory). This scenario arises because:

  1. When the auction is created using the AuctionFactory, the factory contract becomes the owner of the auction contract, as demonstrated in the constructor of the FjordAuction contract:

constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
uint256 _totalTokens
) {
if (_fjordPoints == address(0)) {
revert InvalidFjordPointsAddress();
}
if (_auctionToken == address(0)) {
revert InvalidAuctionTokenAddress();
}
fjordPoints = ERC20Burnable(_fjordPoints);
auctionToken = IERC20(_auctionToken);
@> owner = msg.sender;
auctionEndTime = block.timestamp.add(_biddingTime);
totalTokens = _totalTokens;
}

  1. During the auction creation process in the AuctionFactory, the following code transfers the auctionToken to the newly deployed auction contract:

function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner {
​
address auctionAddress = address(
new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
);
​
// Transfer the auction tokens from the msg.sender to the new auction contract
@> IERC20(auctionToken).transferFrom(msg.sender, auctionAddress, totalTokens);
​
emit AuctionCreated(auctionAddress);
}

However, if no bids are placed, the auctionEnd() function will return the tokens to the factory contract. The problem is that there is no function in the AuctionFactory contract that allows the retrieval of these tokens. This effectively results in the tokens being locked within the factory contract.

Impact

When this issue occurs, the auction tokens could be permanently locked in the factory contract, rendering them inaccessible. This results in a loss of auction tokens for the project, especially if the tokens represent valuable assets.

Tools Used

Manual review.

Recommendations

Implement a method in the AuctionFactory contract that allows the owner to recover the auction tokens in case they are returned due to no bids being placed. This could be done by adding a simple withdrawTokens() function:

function withdrawTokens(address token, uint256 amount) external onlyOwner {
IERC20(token).transfer(owner, amount);
}

Or another solution can be to allow the original auction creator (or another authorized address) to be the owner of the auction contract, instead of defaulting to the factory contract. This could avoid scenarios where tokens get locked in contracts unintentionally.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

If no bids are placed during the auction, the `auctionToken` will be permanently locked within the `AuctionFactory`

An auction with 0 bids will get the `totalTokens` stuck inside the contract. Impact: High - Tokens are forever lost Likelihood - Low - Super small chances of happening, but not impossible

Support

FAQs

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