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

The `FjordAuctionFactory` Does Not Transfer Ownership To `Owner`

Summary

The FjordAuctionFactory is the owner of all created FjordAuctions.

Vulnerability Details

When creating a FjordAuction, the constructor marks the msg.sender as the owner:

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; /// @audit owner_is_msg.sender
auctionEndTime = block.timestamp.add(_biddingTime);
totalTokens = _totalTokens;
}

In the context of creating new auctions via aFjordAuctionFactory, this means the factory itself is given ownership:

function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner {
/// @audit Even though `createAuction` may only be invoked by the `owner`
/// @audit of the `FjordAuctionFactory`, ownership of the created auctions
/// @audit is assigned to the factory itself, and not the factory owner.
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);
}

Impact

If an auction ends with no bids, any tokens remaining on the FjordAuction are designed to be transferred to the owner:

if (totalBids == 0) {
auctionToken.transfer(owner, totalTokens); /// @audit transfer_all_tokens_to_owner
return;
}

For auctions created via the factory, this will result in auctionTokens stuck on the FjordAuctionFactory itself.

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L192C9-L195C10

Tools Used

Manual Review

Recommendations

Either specify the owner of the FjordAuction via constructor argument or add the capability for the owner of the FjordAuctionFactory to rescue tokens.

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.