Summary
In the FjordAuction contract, when an auction ends with no bids, all auction tokens are transferred to the 'owner' address, which is set to the FjordAuctionFactory contract. However, the FjordAuctionFactory contract lacks functionality to withdraw or transfer these tokens, resulting in a permanent lock of the auctioned tokens.
Vulnerability Details
The vulnerability stems from the interaction between FjordAuctionFactory and FjordAuction contracts:
function createAuction(
address auctionToken,
uint256 biddingTime,
uint256 totalTokens,
bytes32 salt
) external onlyOwner {
address auctionAddress = address(
new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
);
}
constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
uint256 _totalTokens
) {
owner = msg.sender;
}
function auctionEnd() external {
if (totalBids == 0) {
auctionToken.transfer(owner, totalTokens);
return;
}
}
The msg.sender in the FjordAuction constructor is the FjordAuctionFactory address, making it the owner. When an auction ends with no bids, tokens are sent to this address.
Due to lack of rescue function for erc20, it stucks in there forever.
POC
In existing test suite auction.t.sol , we need to modify it little bit
//// existing code
+ import "src/FjordAuctionFactory.sol";
contract TestAuction is Test {
using SafeMath for uint256;
FjordAuction public auction;
AuctionFactory public factory;
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");
+ factory = new AuctionFactory(address(fjordPoints));
- auction =
- new FjordAuction(address(fjordPoints), address(auctionToken), biddingTime, totalTokens);
- deal(address(auctionToken), address(auction), totalTokens);
}
and add the following test
function testEndTheAuctionWithNoBidsFundsStuckInFactoryForever() public {
address factoryOwner = factory.owner();
deal(address(auctionToken), factoryOwner, 1000 ether);
vm.startPrank(factoryOwner);
auctionToken.approve(address(factory), 100 ether);
bytes32 salt = keccak256("test");
address auctionAddress = address(uint160(uint256(keccak256(abi.encodePacked(
bytes1(0xff),
address(factory),
salt,
keccak256(abi.encodePacked(
type(FjordAuction).creationCode,
abi.encode(address(fjordPoints), address(auctionToken), 1 days, 100 ether)
))
)))));
factory.createAuction(address(auctionToken), 1 days, 100 ether, salt);
console.log("Auction created:", auctionAddress);
console.log("auction contract balance initial balance:", auctionToken.balanceOf(auctionAddress));
FjordAuction auctionOp = FjordAuction(auctionAddress);
assertEq(auctionToken.balanceOf(auctionAddress), 100 ether, "Auction should have tokens");
assertEq(auctionOp.owner(), address(factory), "Factory should be the owner of the auction");
vm.warp(block.timestamp + 2 days);
auctionOp.auctionEnd();
console.log("auction contract balance after auctionEnd:", auctionToken.balanceOf(auctionAddress));
assertEq(auctionToken.balanceOf(address(factory)), 100 ether, "Tokens should be sent to factory");
assertEq(auctionToken.balanceOf(auctionAddress), 0, "Auction should have no tokens left");
console.log("Factory contract balance after auctionEnd (when no bid):", auctionToken.balanceOf(address(factory)));
}
now run forge test --mt testEndTheAuctionWithNoBidsFundsStuckInFactoryForever -vv in the terminal, it will return following output:
[⠒] Compiling...
[⠃] Compiling 1 files with Solc 0.8.21
[⠊] Solc 0.8.21 finished in 1.79s
Compiler run successful!
Ran 1 test for test/unit/auction.t.sol:TestAuction
[PASS] testEndTheAuctionWithNoBidsFundsStuckInFactoryForever() (gas: 989691)
Logs:
Auction created: 0x4DDe319E73aB36623f26bF9cB4e3A886ede43432
auction contract balance initial balance: 100000000000000000000
auction contract balance after auctionEnd: 0
Factory contract balance after auctionEnd (when no bid): 100000000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.53ms (566.67µs CPU time)
Ran 1 test suite in 152.86ms (1.53ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Loss of funds (auction tokens)
Tools Used
Manual Review Foundry
Recommendations
Here are few ways to make a fix for it:
function claimStuckedERC20(address token, address to, uint256 amount) external onlyOwner {
(bool success, bytes memory data) = token.call(abi.encodeWithSelector(0xa9059cbb, to, amount));
require(
success && (data.length == 0 || abi.decode(data, (bool))),
'FjordAuctionFactory::safeTransfer: transfer failed'
);
}
in FjordAuction update the constructor
constructor(
address _fjordPoints,
address _auctionToken,
+ address _owner,
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;
}
In FjordAuctionFactory update the createAuction function
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}(fjordPoints, auctionToken, owner, 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);
}
