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

Attacker can game the auction and claim more tokens than they should by calling `auctionEnd()` and `bid()` at `block.timestamp == auctionEndTime`

0xnbvc

Vulnerability Details

There exists a race condition in the FjordAuction contract where the auction end time and bidding process can overlap at block.timestamp == auctionEndTime:

FjordAuction.sol#L143-L146

function bid(uint256 amount) external {
-> if (block.timestamp > auctionEndTime) { //@audit-info Can be called at block.timestamp == auctionEndTime
revert AuctionAlreadyEnded();
}

FjordAuction.sol#L181-L184

function auctionEnd() external {
-> if (block.timestamp < auctionEndTime) { //@audit-info Can be called at block.timestamp == auctionEndTime
revert AuctionNotYetEnded();
}

If the auctionEnd() function is called at the exact auctionEndTime and a user calls bid() at the same timestamp, the fjordPoints from the bid won't be accounted for in the multiplier calculation:

FjordAuction.sol#L197

multiplier = totalTokens.mul(PRECISION_18).div(totalBids);

This would lead to an inaccurate distribution of tokens and allow the attacker to claim more tokens than they should.

FjordAuction.sol#L217

uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);

Impact

Attacker can game the auction and claim more tokens than they should, stealing tokens from other users.

Scenario

Setup is totalTokens = 1000, totalBids = 1000

  1. Attacker calls auctionEnd() at block.timestamp == auctionEndTime

  2. multiplier is calculated as 1000 * 1e18 / 1000 = 1e18

  3. Attacker calls bid() at the exact block.timestamp == auctionEndTime with amount = 1000

    1. multiplier should be 1000 * 1e18 / 2000 = 0.5e18, but it has already been calculated as 1e18

  4. Attacker claims their tokens with claimTokens()

  5. Attacker gets 1000 * 1e18 = 1000 tokens

    1. But should gets only 1000 * 0.5e18 = 500 tokens

Recommendations

Modify the bid() and unbid() functions to check if block.timestamp >= auctionEndTime instead of block.timestamp > auctionEndTime.

function bid(uint256 amount) external {
- if (block.timestamp > auctionEndTime) {
+ if (block.timestamp >= auctionEndTime) {
revert AuctionAlreadyEnded();
}
...
}
​
function unbid(uint256 amount) external {
- if (block.timestamp > auctionEndTime) {
+ if (block.timestamp >= auctionEndTime) {
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.