Vulnerability Details
Description
FjordAuction is created via FjordAuctionFactory
File: 2024-08-fjord/src/FjordAuctionFactory.sol
function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner {
address auctionAddress = address(
new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
);
IERC20(auctionToken).transferFrom(msg.sender, auctionAddress, totalTokens);
emit AuctionCreated(auctionAddress);
}
The owner of created FjordAuction is set as msg.sender inside constructor function.
File: 2024-08-fjord/src/FjordAuction.sol
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;
}
As a result, the owner of FjordAuction will always be FjordAuctionFactory contract.
The problem arises when the created auction concludes with zero bids. Anyone can call FjordAuction.auctionEnd() to proceed the auction once block.timestamp exceeds auctionEndTime.
File: 2024-08-fjord/src/FjordAuction.sol
function auctionEnd() external {
if (block.timestamp < auctionEndTime) {
revert AuctionNotYetEnded();
}
if (ended) {
revert AuctionEndAlreadyCalled();
}
ended = true;
emit AuctionEnded(totalBids, totalTokens);
if (totalBids == 0) {
auctionToken.transfer(owner, totalTokens);
return;
}
multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}
In the case of zero bids, all auction tokens are sent back to the owner. However, as mentioned before, the owner of auction contract created from factory is always a factory contract. Thus, auction tokens will be sent back to factory contract.
Unfortunately, FjordAuctionFactory doesn't implement a recovery mechanism to transfer those auction tokens out of factory contract. As a result, those auction tokens from zero bids auction become permanently locked withint the factory contract.
Proof-of-Concept
The following PoC demonstrates that
The owner of Auction instance created via factory is always set as factory contract.
All auction tokens are sent back to factory contract in the case of zero bids auction.
Tokens are not recoverable from factory contract
Steps
Create a new file, factory.t.sol, in 2024-08-fjord/test/unit/ and paste the following test.
Run forge test --match-contract TestAuctionLocked --match-test testAuctionEndWithNoBids -vv
Observe that auction tokens are sent back to factory contract when auction ends with zero bids.
pragma solidity =0.8.21;
import "forge-std/Test.sol";
import "src/FjordAuction.sol";
import "src/FjordAuctionFactory.sol";
import { ERC20BurnableMock } from "../mocks/ERC20BurnableMock.sol";
contract TestAuctionLocked is Test {
FjordAuction public auction;
AuctionFactory public factory;
ERC20BurnableMock public fjordPoints;
ERC20BurnableMock public auctionToken;
address public owner = address(0x1);
uint256 public biddingTime = 1 weeks;
uint256 public totalTokens = 1000 ether;
function setUp() public {
fjordPoints = new ERC20BurnableMock("FjordPoints", "fjoPTS");
auctionToken = new ERC20BurnableMock("AuctionToken", "AUCT");
}
function testAuctionEndWithNoBidsLockedInFactory() public {
factory =
new AuctionFactory(address(fjordPoints));
console.log("@> Deployed factory contract at %s", address(factory));
deal(address(auctionToken), address(this), totalTokens);
auctionToken.approve(address(factory), type(uint).max);
console.log("@> Creating new auction via factory");
vm.recordLogs();
factory.createAuction(
address(auctionToken),
biddingTime,
totalTokens,
keccak256("salt_goodman")
);
Vm.Log[] memory entries = vm.getRecordedLogs();
uint entries_length = entries.length;
auction = FjordAuction(address(uint160(uint256(entries[entries_length-1].topics[1]))));
console.log("@> New auction created at: %s", address(auction));
console.log("@> Assert that factory holds no tokens before auction end");
uint balBefore = auctionToken.balanceOf(address(factory));
assertEq(balBefore, 0);
console.log("@> Assert that auction's owner is factory");
assertEq(address(factory), auction.owner());
console.log("@> Skip time, then call auctionEnd()");
skip(biddingTime);
auction.auctionEnd();
uint balAfter = auctionToken.balanceOf(address(factory));
console.log("@> Assert that factory holds %s tokens after auction end", balAfter);
assertEq(balAfter, totalTokens);
}
}
Impact
Auction tokens from zero-bid auctions are erroneously sent to a factory contract lacking a recovery mechanism. As a result, these auction tokens become permanently locked within the factory contract.
Rationale for Severity
Although it leads to permanently locked funds, it only happens in zero-bids auction.
Hence, Medium severity.
Recommendation
Use owner of factory contract as auction owner.