Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Auction tokens get locked after auctionEnd if totalBids are zero

dianivanov

Summary

When the totalBids of a FjordAuction are zero, and the auctionEnd function is called, then the totalTokens amount from the FjordAuctioncontract are getting locked into the AuctionFactory contract. This is because the AuctionFactoryis the owner of the FjordAuction.

Vulnerability Details

When a FjordAuctioncontract is deployed from the AuctionFactory contract using the createAuction function, the AuctionFactoryis the msg.senderof the FjordAuction contract's constructor:

contract AuctionFactory {
...
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);
}
}
contract FjordAuction {
...
constructor(
address _fjordPoints,
address _auctionToken,
uint256 _biddingTime,
uint256 _totalTokens
) {
...
owner = msg.sender;
...
}
...
}
contract FjordAuction {
...
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);
}
...
}

As a result the AuctionFactorycontract is the owner of any FjordAuction created from createAuction function.

When the totalBids are 0 and the function auctionEnd is called, it is sending a totalTokens amount of auctionTokento the ownerof the FjordAuction, but the owneris AuctionFactoryand it is not deisgned to handle any tokens in its possesion. As a result the tokens are locked.

Proof of concept:

  1. The owner of the AuctionFactory is calling createAuctionfunction to deploy a new FjordAuctionwith a specific totalTokensamount.

  2. auctionEndTimehas passed, there are no bids (totalBids == 0) and someone calls auctionEnd

  3. An amount of totalTokensauction tokens are sent to the AuctionFactory

  4. The AuctionFactorydoes not handle any tokens and they remain locked in the contract.

Proof of code:
Add the following test to test/unit/auction.t.sol

function testAuctionEndWhenNoBidders() public {
address alice = makeAddr("alice");
//give some tokens to alice
deal(address(auctionToken), alice, totalTokens);
console.log("Alice balance: ", auctionToken.balanceOf(alice));
​
vm.startPrank(alice);
​
AuctionFactory auctionFactory = new AuctionFactory(address(fjordPoints));
​
//alice approves the factory to transfer the tokens to it
auctionToken.approve(address(auctionFactory), totalTokens);
bytes32 salt = keccak256(abi.encodePacked(block.timestamp, msg.sender));
//alice creates an auction and we use the event to get it's address
vm.recordLogs();
auctionFactory.createAuction(address(auctionToken), biddingTime, totalTokens, salt);
Vm.Log[] memory logs = vm.getRecordedLogs();
address auctionAddress;
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;
}
}
​
​
console.log("Auction Address:", auctionAddress);
FjordAuction fjordAuction = FjordAuction(auctionAddress);
console.log("---------BEFORE auctionEnd()---------");
console.log("fjordAuction's auctionToken balance: ", auctionToken.balanceOf(address(fjordAuction)));
console.log("alice's auctionToken balance: ", auctionToken.balanceOf(alice));
console.log("auctionFactory's auctionToken balance: ", auctionToken.balanceOf(address(auctionFactory)));
console.log("---------BEFORE auctionEnd()---------");
​
//biddingTime ends and auctionEnd is performed, this sends the tokens to the owner
//but the owner is the AuctionFactory, hence the tokens are locked
skip(biddingTime);
fjordAuction.auctionEnd();
console.log("----------AFTER auctionEnd()---------");
console.log("fjordAuction's auctionToken balance: ", auctionToken.balanceOf(address(fjordAuction)));
console.log("alice's auctionToken balance: ", auctionToken.balanceOf(alice));
console.log("auctionFactory's auctionToken balance: ", auctionToken.balanceOf(address(auctionFactory)));
console.log("----------AFTER auctionEnd()---------");
​
vm.stopPrank();
}

Impact

The tokens are getting locked into the AuctionFactory contract.

Tools Used

Manual review & foundry

Recommendations

Either add the possibility the owner of the FjordAuctionto be passed to the constructor so it can be passed the same as the owner of the AuctionFactory(or any other address who know what to do with the tokens) OR add a withdrawTokens function to the AuctionFactorycallable only by the owner of the factory so the tokens can be transferred and are not getting locked.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 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.

Give us feedback!