Summary
The FjordAuctionFactory.sol contract is used to create new auctions, and it also transfers the auction tokens to the new auction. If someone ends the auction, all auction tokens are sent back to the owner. The owner of the auction is the FjordAuctionFactory contract, which does not have a method to withdraw the auction tokens to the factory contract's owner. Therefore, the tokens are lost.
Vulnerability Details
A given address deploys the FjordAuctionFactory contract. This address becomes the owner of the factory contract.
The createAuction() method is called in order to create a new auction
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 createAuction() transfers the totalTokens amount of auctionToken tokens to the auctionAddress from the msg.sender. Here the msg.sender is the factory contract's owner.
FjordAuction's constructor is called on deploy, which sets the owner to the msg.sender.
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;
}
Let's assume there are no bids and auctionEndTime is reached.
The bid() and unbid() methods will revert, because of auction end check.
FjordAuction.sol
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
The claimTokens() method still can't be called, because the auctionEnd() is not called to end the auction.
FjordAuction.sol
if (!ended) {
revert AuctionNotYetEnded();
}
Someone calls auctionEnd() method from FjordAuction in order to end the auction and calculate the claimable tokens.
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);
}
Now, the claimTokens() will revert, because there are not any bids.
FjordAuction.sol
if (userBids == 0) {
revert NoTokensToClaim();
}
The auctionEnd method will execute the following code snippet.
FjordAuction.sol
if (totalBids == 0) {
auctionToken.transfer(owner, totalTokens);
return;
}
The totalTokens amount of auction tokens will be transferred back to the owner address. The owner of the auction contract is the FjordAuctionFactory contract, which does not have a method to withdraw the auction tokens to the factory contract's owner. Therefore, the tokens are lost.
Impact
The totalTokens amount of auction tokens are lost forever and can't be recovered.
PoC
Import the console.sol in test/unit/auction.t.sol.
import "forge-std/console.sol";
Add the following test case in test/unit/auction.t.sol.
function testAuctionEndWithNoBidsAndTokenLoss() public {
bytes32 salt = keccak256(abi.encodePacked(block.timestamp, owner));
vm.startPrank(owner);
AuctionFactory factory = new AuctionFactory(address(fjordPoints));
FjordToken auctionFjordToken = new FjordToken();
auctionFjordToken.approve(address(factory), totalTokens);
console.log("owner auctionToken balance before creating an auction", auctionFjordToken.balanceOf(owner));
assertEq(auctionFjordToken.balanceOf(owner), 100_000_000 ether);
console.log("factory auctionToken balance before creating an auction", auctionFjordToken.balanceOf(address(factory)));
assertEq(auctionFjordToken.balanceOf(address(factory)), 0);
factory.createAuction(
address(auctionFjordToken),
biddingTime,
totalTokens,
salt
);
console.log("owner auctionToken balance after creating an auction", auctionFjordToken.balanceOf(owner));
assertEq(auctionFjordToken.balanceOf(owner), 100_000_000 ether - totalTokens);
console.log("factory auctionToken balance after creating an auction", auctionFjordToken.balanceOf(address(factory)));
assertEq(auctionFjordToken.balanceOf(address(factory)), 0);
vm.stopPrank();
bytes memory args = abi.encode(address(fjordPoints), address(auctionFjordToken), biddingTime, totalTokens);
bytes memory creationCode = type(FjordAuction).creationCode;
bytes memory bytecode = abi.encodePacked(creationCode, args);
bytes32 hash = keccak256(
abi.encodePacked(
bytes1(0xff),
address(factory),
salt,
keccak256(bytecode)
)
);
address computedAddress = address(uint160(uint256(hash)));
console.log("computedAddress", computedAddress);
FjordAuction newAuction = FjordAuction(computedAddress);
console.log("auction owner", newAuction.owner());
console.log("address(factory)", address(factory));
assertEq(newAuction.owner(), address(factory));
skip(biddingTime);
newAuction.auctionEnd();
console.log("owner auctionToken balance after auction ends", auctionFjordToken.balanceOf(owner));
assertEq(auctionFjordToken.balanceOf(owner), 100_000_000 ether - totalTokens);
console.log("factory auctionToken balance after auction ends", auctionFjordToken.balanceOf(address(factory)));
assertEq(auctionFjordToken.balanceOf(address(factory)), totalTokens);
}
Execute it with forge test -vv --mt testAuctionEndWithNoBidsAndTokenLoss and the result should be similar to the following output:
Ran 1 test for test/unit/auction.t.sol:TestAuction
[PASS] testAuctionEndWithNoBidsAndTokenLoss() (gas: 2551973)
Logs:
owner auctionToken balance before creating an auction 100000000000000000000000000
factory auctionToken balance before creating an auction 0
owner auctionToken balance after creating an auction 99999000000000000000000000
factory auctionToken balance after creating an auction 0
computedAddress 0xe5183f9fc60550E4599bc0125542221a45bF9e2A
auction owner 0x522B3294E6d06aA25Ad0f1B8891242E335D3B459
address(factory) 0x522B3294E6d06aA25Ad0f1B8891242E335D3B459
owner auctionToken balance after auction ends 99999000000000000000000000
factory auctionToken balance after auction ends 1000000000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.78ms (692.38µs CPU time)
Ran 1 test suite in 5.22ms (2.78ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Tools Used
Manual, Foundry
Recommendations
Add an owner argument in FjordAuctionFactory::createAuction() method and in FjordAuction::constructor().
FjordAuctionFactory.sol
...
+ * @param auctionOwner The new auction's owner
*/
function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
- bytes32 salt
+ bytes32 salt,
+ address auctionOwner
) external onlyOwner {
+ if (auctionOwner == address(0)) {
+ revert InvalidAddress();
+ }
+
address auctionAddress = address(
- new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
+ new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens, auctionOwner)
);
...
FjordAuction.sol
...
+ * @param _owner The owner of the auction
*/
constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
- uint256 _totalTokens
+ uint256 _totalTokens,
+ address _owner
) {
if (_fjordPoints == address(0)) {
revert InvalidFjordPointsAddress();
@@ -131,7 +133,7 @@ contract FjordAuction {
}
fjordPoints = ERC20Burnable(_fjordPoints);
auctionToken = IERC20(_auctionToken);
- owner = msg.sender;
+ owner = _owner;
auctionEndTime = block.timestamp.add(_biddingTime);
totalTokens = _totalTokens;
}
...
