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

Users may bid then claim after auction end to drain the entire tokens

arnie

Summary

Users may bid then claim after auction end to drain the entire tokens

Vulnerability Details

The auctionEnd function will end the auction if block.timestamp < auctionEndTime

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

the function will set the ended variable to true. Then the function will find the multiplier by multiplying the total tokens by the precision_18 then dividing by the total bids.

The problem occurs because users are allowed to bid after the auction has ended because of the incorrect use of the inequality to check if the auction has truly ended

let us observe the bid function below...

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
​
bids[msg.sender] = bids[msg.sender].add(amount);
totalBids = totalBids.add(amount);
​
fjordPoints.transferFrom(msg.sender, address(this), amount);
emit BidAdded(msg.sender, amount);
}

as we can see if block.timestamp > auctionEndTime we can still bid, but if we recall from the auctionEnd function, if (block.timestamp < auctionEndTime) then we can call endAuction

this is very problematic... because if we see a scenario where block.timestamp = auctionEndTime, both inequalities will pass for auctionEnd function and bid function.

This allows for the auction to be ended, but in the same block the user can then call bid then the next transaction he can call claim.

because the auctionEnd set the multiplier and the variableended to true, the user can bid with large amount and thencall claim in order to drain the entire tokens and cause others to be able to claim 0. This happens because the users bids was not used when accounting the multiplier in the endAuction function

Let us explain this step by step below...

POC

  1. auction ends because block.timestamp = auctionEndTime and user call endAuction

  2. the total bids is 100

  3. the token amount is 100

  4. The multiplier is set and ended is set to true

  5. the multiplier will be essentially 1, meaning that 1 point bid can claim 1 token

  6. a user may still bid and he bids a large amount, he bids 100 points after the auction ended

  7. the user may now claim and because the multiplier is 1 he can claim 100 tokens

  8. the contract is drained of all tokens and no other of the bidders can claim

Relevant Code Links

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L143

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L181

Impact

Direct theft of users funds

Tools Used

manual review

Recommendations

add the following check in bid and unbid functions to ensure a user may not bid after auction end
if (ended) { revert AuctionEndAlreadyCalled();

Updates

Lead Judging Commences

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