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

Permanent loss of auction tokens due to wrong configuration

Summary

When creating an auction using the factory contract, the factory address is set as the owner of the auction. In case an auction ends with no bidders, the tokens are transferred to the factory contract, which cannot move the tokens later.

Vulnerability Details

A user, presumably a Fjord admin can create an auction using FjordAuctionFactory::createAuction which deploys a new FjordAuction contract where the owner is set to msg.sender (factory) and cannot be changed.

The owner variable is used solely as the recipient of the auction tokens if there are no bidders after the auction ends.

The factory doesn’t implement any functionality to transfer ERC20 tokens, resulting in a permanent loss of these tokens within the factory contract.

Proof of Concept

The following PoC demonstrates that the factory contract receives the auction tokens if no bids are placed.

import "src/FjordAuctionFactory.sol";
// test/unit/auction.t.sol
function testStuckTokens() public {
deal(address(auctionToken), owner, totalTokens);
vm.startPrank(owner);
AuctionFactory factory = new AuctionFactory(address(fjordPoints));
auctionToken.approve(address(factory), totalTokens);
factory.createAuction(address(auctionToken), 2 days, totalTokens, "");
vm.stopPrank();
// predeterministic address of the auction
FjordAuction auction = FjordAuction(0x963EaaA857aF59A72Db5581704f3462bC3924A26);
assertEq(factory.owner(), owner);
assertEq(auction.owner(), address(factory));
skip(2 days);
// ends auction and transfers tokens to the factory
uint256 factoryBalanceBefore = auctionToken.balanceOf(address(factory));
auction.auctionEnd();
uint256 factoryBalanceAfter = auctionToken.balanceOf(address(factory));
assertGt(factoryBalanceAfter, factoryBalanceBefore);
}

Impact

Permanent loss of auction tokens

Tools Used

Manual Review, Foundry

Recommendations

Consider adding an _owner parameter to the constructor in FjordAuction.sol and ensure the address can receive and spend the tokens.

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

Alternatively, you can add token transfer functionality to the FjordAuctionFactory, which would only be callable by the owner:

function transferTokens(IERC20 token, address receiver, uint256 amount) external onlyOwner {
token.transfer(receiver, amount);
}
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.