Summary
In the current implementation, whenever a new auction event starts, factory owner calls createAuction() function in the factory. At this time the auction is started automatically. If there is no participation in the auction, token can be claimable by the owner but due to wrong owner configuration it's not possible.
Vulnerability Details
Let's examine the code step by step. If the owner wants to create a new auction event, they need to call the createAuction() function in the factory contract.
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);
}
Using the CREATE2 operation, a new auction contract is deployed, and auction tokens are sent to it. The problem arises in the constructor of the auction contract. It sets the owner using the msg.sender keyword, which causes the factory to become the owner of the auction.
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;
}
The owner of the auction is relevant in situations where there are no participants in the auction. If there is no participation, the owner can claim the tokens.
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);
}
The tokens will be sent to the factory contract, but the withdraw() function implementation is missing in the factory contract.
Coded PoC
pragma solidity =0.8.21;
import { ERC20 } from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import { ERC20Burnable } from
"lib/openzeppelin-contracts/contracts/token/ERC20/extensions/ERC20Burnable.sol";
contract ERC20BurnableMock is ERC20, ERC20Burnable {
constructor(string memory name_, string memory symbol_) ERC20(name_, symbol_) { }
function deal(address account, uint256 amount) external {
_mint(account, amount);
}
}
pragma solidity =0.8.21;
import "forge-std/Test.sol";
import "src/FjordAuction.sol";
import { ERC20BurnableMock } from "../mocks/ERC20BurnableMock.sol";
import { SafeMath } from "lib/openzeppelin-contracts/contracts/utils/math/SafeMath.sol";
import "src/FjordAuctionFactory.sol";
contract TestAuction is Test {
using SafeMath for uint256;
ERC20BurnableMock public fjordPoints;
ERC20BurnableMock public auctionToken;
AuctionFactory public factory;
address public owner = address(0x1);
uint256 public biddingTime = 1 weeks;
uint256 public totalTokens = 1000 ether;
function setUp() public {
vm.startPrank(owner);
fjordPoints = new ERC20BurnableMock("FjordPoints", "fjoPTS");
auctionToken = new ERC20BurnableMock("AuctionToken", "AUCT");
factory = new AuctionFactory(address(fjordPoints));
vm.stopPrank();
}
function test_deployAuction() public {
console.log("Owner address: ", owner);
console.log("Address of factory: ", address(factory));
bytes32 salt = keccak256(abi.encode("First Auction Salt"));
vm.startPrank(owner);
auctionToken.deal(owner, totalTokens);
auctionToken.approve(address(factory), totalTokens);
address auction = factory.createAuction(address(auctionToken), biddingTime, totalTokens, salt);
vm.stopPrank();
address _auctionOwner = FjordAuction(auction).owner();
console.log("Auction owner: ", _auctionOwner);
assertEq(_auctionOwner, owner);
}
}
Note: In createAuction() function, I made a small modification, now it returns the address of the auction
function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner returns (address) {
address auctionAddress = address(
new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
);
IERC20(auctionToken).transferFrom(msg.sender, auctionAddress, totalTokens);
emit AuctionCreated(auctionAddress);
&> return auctionAddress;
}
The Outputs
Ran 1 test for test/invariant/auction_factory.t.sol:TestAuction
[FAIL. Reason: assertion failed: 0xc051134F56d56160E8c8ed9bB3c439c78AB27cCc != 0x0000000000000000000000000000000000000001] test_deployAuction() (gas: 861999)
Logs:
Owner address: 0x0000000000000000000000000000000000000001
Address of factory: 0xc051134F56d56160E8c8ed9bB3c439c78AB27cCc
Auction owner: 0xc051134F56d56160E8c8ed9bB3c439c78AB27cCc
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.25ms (337.94µs CPU time)
Ran 1 test suite in 19.15ms (1.25ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Impact
Loss of funds
-- The tokens will be stuck in the factory contract
Tools Used
Manual Review
Recommendations
Implementing a withdraw() function in the factory contract will solve the problem.
function withdraw(address token, uint256 amount) external onlyOwner {
IERC20(token).transfer(address(owner), amount);
}
