DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: medium
Valid

Auction created from factory contract uses wrong owner address could lead to permanently locked funds in zero-bids auction

Vulnerability Details

Description

FjordAuction is created via FjordAuctionFactory

File: 2024-08-fjord/src/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)
);
// Transfer the auction tokens from the msg.sender to the new auction contract
IERC20(auctionToken).transferFrom(msg.sender, auctionAddress, totalTokens);
emit AuctionCreated(auctionAddress);
}

The owner of created FjordAuction is set as msg.sender inside constructor function.

File: 2024-08-fjord/src/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;
}

As a result, the owner of FjordAuction will always be FjordAuctionFactory contract.

The problem arises when the created auction concludes with zero bids. Anyone can call FjordAuction.auctionEnd() to proceed the auction once block.timestamp exceeds auctionEndTime.

File: 2024-08-fjord/src/FjordAuction.sol
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);
// Burn the FjordPoints held by the contract
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}

In the case of zero bids, all auction tokens are sent back to the owner. However, as mentioned before, the owner of auction contract created from factory is always a factory contract. Thus, auction tokens will be sent back to factory contract.

Unfortunately, FjordAuctionFactory doesn't implement a recovery mechanism to transfer those auction tokens out of factory contract. As a result, those auction tokens from zero bids auction become permanently locked withint the factory contract.

Proof-of-Concept

The following PoC demonstrates that

  • The owner of Auction instance created via factory is always set as factory contract.

  • All auction tokens are sent back to factory contract in the case of zero bids auction.

  • Tokens are not recoverable from factory contract

Steps

  1. Create a new file, factory.t.sol, in 2024-08-fjord/test/unit/ and paste the following test.

  2. Run forge test --match-contract TestAuctionLocked --match-test testAuctionEndWithNoBids -vv

  3. Observe that auction tokens are sent back to factory contract when auction ends with zero bids.

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
import "forge-std/Test.sol";
import "src/FjordAuction.sol";
import "src/FjordAuctionFactory.sol";
import { ERC20BurnableMock } from "../mocks/ERC20BurnableMock.sol";
contract TestAuctionLocked is Test {
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");
}
function testAuctionEndWithNoBidsLockedInFactory() public {
factory =
new AuctionFactory(address(fjordPoints));
console.log("@> Deployed factory contract at %s", address(factory));
deal(address(auctionToken), address(this), totalTokens);
auctionToken.approve(address(factory), type(uint).max);
console.log("@> Creating new auction via factory");
vm.recordLogs();
factory.createAuction(
address(auctionToken),
biddingTime,
totalTokens,
keccak256("salt_goodman")
);
Vm.Log[] memory entries = vm.getRecordedLogs();
uint entries_length = entries.length;
auction = FjordAuction(address(uint160(uint256(entries[entries_length-1].topics[1]))));
console.log("@> New auction created at: %s", address(auction));
console.log("@> Assert that factory holds no tokens before auction end");
uint balBefore = auctionToken.balanceOf(address(factory));
assertEq(balBefore, 0);
console.log("@> Assert that auction's owner is factory");
assertEq(address(factory), auction.owner());
console.log("@> Skip time, then call auctionEnd()");
skip(biddingTime);
auction.auctionEnd();
uint balAfter = auctionToken.balanceOf(address(factory));
console.log("@> Assert that factory holds %s tokens after auction end", balAfter);
assertEq(balAfter, totalTokens);
}
}

Impact

Auction tokens from zero-bid auctions are erroneously sent to a factory contract lacking a recovery mechanism. As a result, these auction tokens become permanently locked within the factory contract.

Rationale for Severity

Although it leads to permanently locked funds, it only happens in zero-bids auction.

Hence, Medium severity.

Recommendation

Use owner of factory contract as auction owner.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

If no bids are placed during the auction, the `auctionToken` will be permanently locked within the `AuctionFactory`

An auction with 0 bids will get the `totalTokens` stuck inside the contract. Impact: High - Tokens are forever lost Likelihood - Low - Super small chances of happening, but not impossible

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.