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

Inappropriate owner assignment leads to loss of funds

0xnilesh

Summary

When an auction is created using the AuctionFactory contract, the FjordAuction contract assigns the Factory contract as its owner. If the auction ends with no bids, the auction tokens are returned to the Factory contract, which results in the tokens being stuck there with no mechanism to retrieve them.

Vulnerability Details

When the owner of the AuctionFactory contract creates an auction using the createAuction function, a new FjordAuction contract is deployed. This new auction contract sets the msg.sender (the Factory contract) as its owner. Consequently, the Factory contract becomes the owner of the auction contract.

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;
}

When the auction ends without any bids, the auction tokens are transferred back to the owner of the contract, which is the Factory contract. Since the Factory contract lacks a mechanism to recover these tokens, they remain permanently stuck.

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;
}

Impact

The funds become irretrievable, effectively getting stuck in the Factory contract.

Proof of Concept (PoC)

Add the following code snippet to the auction.t.sol file, and import the Factory contract into the same file. This PoC demonstrates how the auction contract assigns the Factory contract's address as its owner, instead of the EOA (Externally Owned Account) that owns the Factory contract.

function getAuctionContractFromLog(Vm.Log[] memory logs) internal pure returns (address) {
address auctionContractAddr;
for (uint256 i = 0; i < logs.length; i++) {
if (logs[i].topics[0] == keccak256("AuctionCreated(address)")) {
auctionContractAddr = address(uint160(uint256(logs[i].topics[1])));
}
}
return auctionContractAddr;
}
​
function test_bug() public {
AuctionFactory factory;
factory = new AuctionFactory(address(fjordPoints));
deal(address(auctionToken), address(this), totalTokens);
vm.recordLogs();
auctionToken.approve(address(factory), totalTokens);
factory.createAuction(address(auctionToken), biddingTime, totalTokens, '0x123');
​
address auctionAddr = getAuctionContractFromLog(vm.getRecordedLogs());
​
FjordAuction auctionContract = FjordAuction(auctionAddr);
​
skip (1 weeks);
vm.prank(address(1));
auctionContract.auctionEnd();
assertEq(address(factory), auctionContract.owner());
}

Tools Used

Manual analysis.

Recommendations

To prevent this issue, pass an owner address parameter to the auction contract during its creation and assign this address as the owner instead of using msg.sender.

Updates

Lead Judging Commences

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