Summary
The FjordAuction.sol contract initializes its owner address during construction. This owner address is used to send the totalTokens back at the end of the auction in the case that there are no bids. However, when creating a new FjordAuction through the FjordAuctionFactory contract, the FjordAuction's owner will be set to be the FjordAuctionFactory address. Sending any tokens to that address will lock them forever, since there is no withdrawal mechanism implemented.
Vulnerability Details
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;
}
We see that the owner of the FjordAuction contract is set to the address that deploys the contract. Creating a new auction through the FjordAuctionFactory contract, makes FjordAuctionFactory's address the deployer of the contract and the owner of the FjordAuctions it deploys.
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);
}
If there are 0 bids in the auction, when it ends it sends all the tokens to the owner. Any tokens sent to the FjordAuctionFactory will be lost, because there is no way to withdraw/recover any tokens from this contract.
Impact
In case there are 0 bids in an auction, all the tokens of the auction will be sent to FjordAuctionFactory's address and will be lost forever.
Proof Of Concept
Create a new file test/unit/auctionFactory.t.sol and paste the following code:
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";
contract TestAuctionFactory is Test {
using SafeMath for uint256;
AuctionFactory public auctionFactory;
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");
vm.startPrank(owner);
auctionFactory = new AuctionFactory(address(fjordPoints));
vm.stopPrank();
deal(address(auctionToken), owner, totalTokens);
}
function testAuctionOwner() public {
bytes32 salt = "1";
vm.startPrank(owner);
auctionToken.approve(address(auctionFactory), totalTokens);
address auction =
auctionFactory.createAuction(address(auctionToken), biddingTime, totalTokens, salt);
vm.stopPrank();
vm.assertEq(FjordAuction(auction).owner(), address(auctionFactory));
assert(FjordAuction(auction).owner() != owner);
}
}
Also, the FjordAuctionFactory.sol file was modified for this test so I could recover the auctionAddress after creating the new FjordAuction contract:
FjordAuctionFactory.sol
function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
- ) external onlyOwner {
+ ) external onlyOwner returns (address) {
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);
+ return auctionAddress;
}
Tools Used
Manual Review
Recommendations
During the FjordAuction construction, add another address for the correct owner:
FjordAuction.sol
constructor(
+ address _owner,
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;
+ owner = _owner;
auctionEndTime = block.timestamp.add(_biddingTime);
totalTokens = _totalTokens;
}
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)
+ new FjordAuction{ salt: salt }(owner, 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);
}
