Summary
The AuctionFactory::createAuction() function is used to create the FjordAuction contract. If no bids are placed, the auctionToken will remain permanently locked within the AuctionFactory contract.
Vulnerability Details
The AuctionFactory::createAuction() function is responsible for creating the FjordAuction contract. The ownership of the newly created FjordAuction contract is assigned to the AuctionFactory contract itself. If no bids are placed during the auction, the funds are returned to the AuctionFactory contract when AuctionFactory::auctionEnd() is called. However, since the AuctionFactory contract lacks an implementation for transferring ERC20 tokens, the auctionToken becomes permanently locked within the AuctionFactory contract.
The following code snippet illustrates the issue. When the owner calls AuctionFactory::createAuction() to deploy the FjordAuction contract, the msg.sender in the FjordAuction::constructor() is the AuctionFactory contract, resulting in owner == AuctionFactory.
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);
}
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;
}
Since the AuctionFactory contract lacks an implementation for transferring ERC20 tokens, the auctionToken is permanently locked within the AuctionFactory contract.
2024-08-fjord$ forge inspect AuctionFactory methodIdentifiers
{
"createAuction(address,uint256,uint256,bytes32)": "56c0703a",
"fjordPoints()": "eb702257",
"owner()": "8da5cb5b",
"setOwner(address)": "13af4035"
}
Poc
Please place the PoC code into the /test directory and execute it:
pragma solidity =0.8.21;
import "../src/FjordStaking.sol";
import { FjordPoints } from "../src/FjordPoints.sol";
import { Test,Vm,console } from "forge-std/Test.sol";
import { MockERC20 } from "solmate/test/utils/mocks/MockERC20.sol";
import { FjordPointsMock } from "./mocks/FjordPointsMock.sol";
import { ISablierV2LockupLinear } from "lib/v2-core/src/interfaces/ISablierV2LockupLinear.sol";
import { ISablierV2Lockup } from "lib/v2-core/src/interfaces/ISablierV2Lockup.sol";
import { Broker, LockupLinear } from "lib/v2-core/src/types/DataTypes.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ud60x18 } from "@prb/math/src/UD60x18.sol";
import "lib/v2-core/src/libraries/Errors.sol";
import {AuctionFactory} from "../src/FjordAuctionFactory.sol";
import {FjordAuction} from "../src/FjordAuction.sol";
contract Poc is Test {
AuctionFactory public auctionFactory;
FjordPoints public fjordPoints;
MockERC20 public auctionToken;
address public owner = makeAddr("owner");
function setUp() public {
auctionToken = new MockERC20("ATOKEN","ATOKEN",18);
vm.startPrank(owner);
fjordPoints = new FjordPoints();
auctionFactory = new AuctionFactory(address(fjordPoints));
vm.stopPrank();
assertEq(owner,auctionFactory.owner());
assertEq(owner,fjordPoints.owner());
auctionToken.mint(owner,100e18);
assertEq(auctionToken.balanceOf(owner),100e18);
}
function test_AuctionFactory_createAuction() public {
address auctionAddress = owner_createAuction();
console.log("auctionAddress:",auctionAddress);
console.log("owner:",owner);
console.log("The owner of auctionFactory:",auctionFactory.owner());
console.log("The owner of auctionAddress:",FjordAuction(auctionAddress).owner());
console.log("auctionFactory address:",address(auctionFactory));
uint256 auctionFactory_auctionTokenBalance_before_auctionEnd = auctionToken.balanceOf(address(auctionFactory));
vm.warp(11);
FjordAuction(auctionAddress).auctionEnd();
uint256 auctionFactory_auctionTokenBalance_after_auctionEnd = auctionToken.balanceOf(address(auctionFactory));
uint amount = auctionFactory_auctionTokenBalance_after_auctionEnd - auctionFactory_auctionTokenBalance_before_auctionEnd;
console.log("The amount of auctionToken received by the auctionFactory contract:",amount);
assertEq(amount,10e18);
}
function owner_createAuction() public returns (address auctionAddress) {
vm.startPrank(owner);
auctionToken.approve(address(auctionFactory),10e18);
vm.recordLogs();
auctionFactory.createAuction(address(auctionToken),10,10e18,"");
vm.stopPrank();
Vm.Log[] memory logs = vm.getRecordedLogs();
for (uint256 i = 0; i < logs.length; i++) {
if (logs[i].topics[0] == keccak256("AuctionCreated(address)")) {
auctionAddress = address(uint160(uint256(logs[i].topics[1])));
break;
}
}
}
}
Code Snippet
https://github.com/Cyfrin/2024-08-fjord/blob/3a78c456b39c799f64c9c2510992584ada3516a0/src/FjordAuctionFactory.sol#L52-L66
https://github.com/Cyfrin/2024-08-fjord/blob/3a78c456b39c799f64c9c2510992584ada3516a0/src/FjordAuction.sol#L120-L137
Impact
If no bids are placed during the auction, the auctionToken will be permanently locked within the AuctionFactory contract due to the lack of an ERC20 transfer implementation.
Tools Used
Manual Review
Recommendations
Consider adding an _owner parameter in the FjordAuction::constructor() to allow the user to initialize the owner. Additionally, modify AuctionFactory::createAuction() to pass msg.sender as a parameter to ensure that the owner of the auction contract is the caller of the FjordAuction::constructor() function.
// AuctionFactory::createAuction()
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 }(msg.sender, 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);
}
// FjordAuction::constructor()
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;
}
