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

When there is `no` bidder in auctionContract then token is transferred to `factoryContract` instead of `owner`

Summary

When there is no bidder in the auctionContract then auctionToken is transferred to factoryContract instead of real owner

Vulnerability Details

Only owner can create auction using auctionFactory:createAuction(), after transferring amount of auctionToken to auctionContract. Also owner of auctionContract is set to msg.sender in constructor, which is factoryContract because it was factoryContract who deploys the auctionContract.

constructor(
...
) {
...
owner = msg.sender;
...
}

When an auction ends and if there is no bidder then auctionToken is transferred to owner of the auctionContract

function auctionEnd() external {
...
if (totalBids == 0) {
@> auctionToken.transfer(owner, totalTokens);
return;
}
...
}

owner of auctionContract is factoryContract(which sets in constructor). As result, auctionTokens are transferred to factoryContract.

Now the problem is, there is no withdraw method in factoryContract, from where owner can withdraw those tokens. As result, those auctionTokens will stuck in factoryContract forever.

//Here is PoC

Run this test in auction.t.sol

function setUp() public {
fjordPoints = new ERC20BurnableMock("FjordPoints", "fjoPTS");
auctionToken = new ERC20BurnableMock("AuctionToken", "AUCT");
vm.prank(owner);
auctionFactory = new AuctionFactory(address(fjordPoints));
}
function testWrongOwnerUsed() public {
//Owner creating Auction using AuctionFactory
vm.startPrank(owner);
deal(address(auctionToken), owner, totalTokens);
auctionToken.approve(address(auctionFactory), totalTokens);
//Note: In auctionFactory, I've returned auction contract address, just for testing
address auctionAddress = auctionFactory.createAuction(
address(auctionToken), biddingTime, totalTokens, bytes32("salt")
);
auction = FjordAuction(auctionAddress);
vm.stopPrank();
//No user bid for auction token
vm.warp(block.timestamp + biddingTime + 1);
//Auction is closed ie Ended
auction.auctionEnd();
//Owner get 0 token but factory get all
assertEq(auctionToken.balanceOf(owner), 0);
assertEq(auctionToken.balanceOf(address(auctionFactory)), totalTokens);
}

Impact

auctionTokens will be stuck in factoryContract when there is no bidder as there is no way to withdraw it

Tools Used

Manual Review

Recommendations

While creating auction in factoryContract, send address on owner and use that owner address in constructor of auctionContract instead of msg.sender

//Factory contract
address auctionAddress = address(
- new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens)
);
address auctionAddress = address(
+ new FjordAuction{ salt: salt }(fjordPoints, auctionToken, biddingTime, totalTokens, owner)
);
//Auction contract
constructor(
...
+ address owner
) {
...
- owner = msg.sender;
+ owner = 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.