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

`auctionFactory` as `owner` of `FjordAuction` Results in Loss of Funds

Summary

The misconfiguration of the owner in the FjordAuction contract, being set to the AuctionFactory contract instead of an admin address, will cause a loss of funds. If no bids are placed and the auction ends, all funds are transferred to the AuctionFactory contract, resulting in tokens being stuck and inaccessible to the intended owner.

Vulnerability Details

When the FjordAuction contract is deployed using the AuctionFactory::createAuction function, the owner of the FjordAuction contract is incorrectly set to the AuctionFactory contract. This creates a critical vulnerability where, in the event of no bids being placed, the tokens intended for the auction owner (admin) are transferred back to the auctionFactory contract. This misconfiguration prevents the retrieval of tokens, leading to a permanent loss of funds.

Vulnerability Scenario:

  1. The AuctionFactory::createAuction function is called to deploy a new FjordAuction contract.

  2. The owner of the deployed FjordAuction contract is automatically set to the AuctionFactory contract.

  3. The auction receives no bids, and the auction is ended.

  4. As per the current contract logic, the tokens are transferred to the AuctionFactory contract instead of the intended admin.

  5. The tokens are now stuck in the AuctionFactory contract, resulting in a loss of funds, as there is no mechanism to retrieve them.

POC:
Copy this file and run the test using forge test --match-test testAuctionOwner -vv.

Impact

The auction owner suffers loss of all tokens locked in the auction due to the inability to retrieve them from the AuctionFactory contract.

Tools Used

Manual Review

Recommendations

To mitigate this issue, add an owner input parameter to the AuctionFactory::createAuction function and modify the constructor of the FjordAuction contract to accept this owner parameter.

Add these lines of code in AuctionFactory::createAuction function:

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

Add these lines of code in FjordAuction's constructor:

constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
-- uint256 _totalTokens
++ uint256 _totalTokens,
++ address _owner
) {
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.