Summary
If an action with no bids is ended, the auction token is send to the owner of the auction contract 1. The owner of the auction contract is the auction factory. As this contract can not handle any ERC20, all tokens would be lost.
Vulnerability Details
After an auction ends, anyone can call auctionEnd. If there are no valid bids (totalBids is zero, L192) the auctionToken is transferred to the owner of the auction.
The auction is deployed as a Factory contract by the FjordAuctionFactory.sol. The auction sets the owner of the auction to the msg.sender. Therefor the owner of the auction is the FjordAuctionFactory. The auction tokens would be send to the factory. The factory has no function to forward the tokens to the original owner, so the tokens are lost.
Impact
If an auction with no bids ends, all tokens put to auction will get send to the factory and are not accessible.
Tools Used
manual review
POC
The POC was added to the existing auction.t.sol.
@@ -3,6 +3,7 @@ pragma solidity =0.8.21;
import "forge-std/Test.sol";
import "src/FjordAuction.sol";
+import "src/FjordAuctionFactory.sol";
import { ERC20BurnableMock } from "../mocks/ERC20BurnableMock.sol";
import { SafeMath } from "lib/openzeppelin-contracts/contracts/utils/math/SafeMath.sol";
@@ -198,6 +199,38 @@ contract TestAuction is Test {
assertEq(balAfter - balBefore, totalTokens);
}
+ function testAuctionEndWithNoBidsFactory_poc() public {
+ // mint tokens
+ deal(address(auctionToken), address(owner), totalTokens);
+
+ // setup factory and create auction
+ vm.startPrank(owner);
+ AuctionFactory factory = new AuctionFactory(address(fjordPoints));
+ auctionToken.approve(address(factory), totalTokens);
+ vm.recordLogs(); // see below
+ factory.createAuction(address(auctionToken), biddingTime, totalTokens, bytes32(0));
+ vm.stopPrank();
+
+ // use vm to read the new auction address from event
+ Vm.Log[] memory entries = vm.getRecordedLogs();
+ assertEq(entries[2].topics[0], keccak256("AuctionCreated(address)"));
+ // event AuctionCreated(address indexed auctionAddress);
+ // => address is at topic 1
+ FjordAuction newAuction = FjordAuction(address(uint160(uint256(entries[2].topics[1]))));
+
+ skip(biddingTime);
+
+ // end auction and fetch balances
+ uint256 balBefore = auctionToken.balanceOf(owner);
+ newAuction.auctionEnd();
+ uint256 balAfter = auctionToken.balanceOf(owner);
+ uint256 balFactory = auctionToken.balanceOf(address(factory));
+
+ assertEq(balBefore, 0); // all tokens are in the auction
+ assertEq(balAfter, 0); // owner does not get a refund
+ assertEq(balFactory, totalTokens); // all tokens are in the factory
+ }
+
function testUnbidToZero() public {
address bidder = address(0x2);
uint256 bidAmount = 100 ether;
Recommendations
Consider making the ownership of the Auction.sol transferable and transferring it to the msg.sender after creating a new auction:
@@ -52,6 +52,16 @@ contract FjordAuction {
* @notice Thrown when a user attempts to withdraw an amount greater than their current bid.
*/
error InvalidUnbidAmount();
+
+ /**
+ * @notice Thrown when a disallowed caller attempts to execute a function that is restricted to specific addresses.
+ */
+ error CallerDisallowed();
+
+ /**
+ * @notice Thrown when an invalid address (e.g., zero address) is provided to a function or during initialization.
+ */
+ error InvalidAddress();
/// @notice The FjordPoints token contract.
ERC20Burnable public fjordPoints;
@@ -136,6 +146,12 @@ contract FjordAuction {
totalTokens = _totalTokens;
}
+ function setOwner(address _newOwner) external {
+ if (msg.sender != owner) revert CallerDisallowed();
+ if (_newOwner == address(0)) revert InvalidAddress();
+ owner = _newOwner;
+ }
+
/**
* @notice Places a bid in the auction.
* @param amount The amount of FjordPoints to bid.
@@ -62,6 +62,8 @@ contract AuctionFactory {
// Transfer the auction tokens from the msg.sender to the new auction contract
IERC20(auctionToken).transferFrom(msg.sender, auctionAddress, totalTokens);
+ FjordAuction(auctionAddress).setOwner(msg.sender);
+
emit AuctionCreated(auctionAddress);
}
}
