DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: medium
Valid

Auctioned tokens become stuck in FjordAuctionFactory is there are no bids

Summary

If an auction ends with no bids the auctioned tokens will be stuck in FjordAuctionFactory because it is the owner of the FjordAuction contract.

Vulnerability Details

When the bidding time passes someone needs to call FjordAuction::auctionEnd() to end the auction and enable users to claim their share of the auctioned token. The problem arises when there are no bids totalBids = 0 because then the tokens are transferred to the owner of the auction, however this owner is not the expected one. We can see below that when totalBids = 0 the auctioned tokens are transferred to the owner:

function auctionEnd() external {
//...
if (totalBids == 0) {
@-> auctionToken.transfer(owner, totalTokens);
return;
}
//...
}

And if we look at the place where that owner is set we come to the constructor of FjordAuction contract:

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

This means that whoever deployed this auction contract will be the owner of it.
Now, the problem is that this owner is actualy the AuctionFactory contract because it is the deployer of all the auctions as it can be seen in AuctionFactory::createAuction():

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);
}

Because of this, the FjordAuction::owner will be the AuctionFactory contract and when an auction ends with no bids the auctioned tokens are transferred to the AuctionFactory contract and they end up stuck there because that contract doesn't implement any functions that allow the withdrawals of such tokens.

Impact

Stuck tokens in AuctionFactory. This means the actual owner of the auction is left with no tokens even when there are no bids.

Tools Used

Manual Review

Recommendations

In FjordAuction set the owner variable through the constructor arguments and to msg.sender. It would look like this:

constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
uint256 _totalTokens,
+ address _owner
) {
if (_fjordPoints == address(0)) {
revert InvalidFjordPointsAddress();
}
if (_auctionToken == address(0)) {
revert InvalidAuctionTokenAddress();
}
+ if (_owner == address(0)) {
+ revert InvalidOwnerAddress();
+ }
fjordPoints = ERC20Burnable(_fjordPoints);
auctionToken = IERC20(_auctionToken);
- owner = msg.sender;
+ owner = _owner;
auctionEndTime = block.timestamp.add(_biddingTime);
totalTokens = _totalTokens;
}
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.