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

`auctionToken` locked if the action ends with no bidders

Summary

As stated in AuctionFactory::actionEnd, all actionTokens will be transferred to owner(AuctionFactory) if no bids were placed.
Since we have no sweep mechanism to recover tokens from AuctionFactory, it will get locked permenantly.

Vulnerability Details

The AuctionFactory::createAuction() function is tasked with creating the FjordAuction contract. Ownership of the newly created FjordAuction contract is assigned to the AuctionFactory contract itself. If no bids are received during the auction, funds are returned to the AuctionFactory contract upon calling AuctionFactory::auctionEnd(). However, due to the lack of an implementation for transferring ERC20 tokens within the AuctionFactory contract, the auctionToken remains permanently locked there.

The following code snippet demonstrates the issue: when the owner invokes AuctionFactory::createAuction() to deploy the FjordAuction contract, the msg.sender in the FjordAuction::constructor() is the AuctionFactory contract, making owner == AuctionFactory.

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

In FjordAuction::constructor():

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

Impact

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

Tools Used

Manual Review

Recommendations

Consider including an explicit owner parameter in auction create process.

function createAuction(
+ address owner,
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner {
address auctionAddress = address(
- new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
+ new FjordAuction{ salt: salt }(owner, 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);
}
constructor(
+ address _owner,
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;
+ owner = _owner;
auctionEndTime = block.timestamp.add(_biddingTime);
totalTokens = _totalTokens;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months 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.