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

Wrong comparison operator used

preslavxyz

Summary

Wrong comparison operator used

Vulnerability Details

After diving into the FjordAuction contract in FjordAuction.sol we can see that the contract has a bid function.

/**
* @notice Places a bid in the auction.
* @param amount The amount of FjordPoints to bid.
*/
function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) { // @audit L-02 should be `>=`
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);
}

This function should not allow a user to bid when an auction has already ended. However, because of the comparison operator used > user has a minimal chance to bid at the exact time when the auction ends. The same issue exists also in the unbid function.

Impact

A bid could technically be placed at the exact time the auction is supposed to end, which violates the intended rules of the auction. This could lead to inconsistencies or disputes in the auction process.

Code Snippet

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

Tools Used

Manual Review

Recommendations

The new if statement shoud look like this:

if (block.timestamp >= auctionEndTime) {
revert AuctionAlreadyEnded();
}
Updates

Lead Judging Commences

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

Appeal created

0xbrivan2 Judge
9 months ago
inallhonesty Lead Judge
9 months ago
inallhonesty Lead Judge 9 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.