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

`FjordAuction` Is Vulnerable To Off-By-One Error

auditweiler

Summary

The FjordAuction uses incorrect comparators which makes them vulnerable to timestamp-based attacks.

Vulnerability Details

The FjordAuction allows bidders to bid and unbid whilst the auction is ongoing, however once the auction's bidding timeframe has elapsed, it should not be possible for buyers to modify their positions, and the outcome of the auction should be respected.

However, these controls are implemented incorrectly:

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) { /// @audit can_bid_at_exactly_end_time
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);
}

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

/**
* @notice Ends the auction and calculates claimable tokens for each bidder based on their bid proportion.
*/
function auctionEnd() external {
if (block.timestamp < auctionEndTime) { /// @audit can_close_auction_at_exactly_end_time
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);
}

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

Due to these incorrect comparisons, it is possible for a user to bid on a closed auction at precisely at the end time (i.e. by incentivising the vulnerable block.timestamp to equal auctionEndTime via PBS, since block proposers can influence the block timestamp).

An attacker can trigger auctionEnd at precisely the auctionEndTime, and place a bid on the auction within the same block. This would allow the attacker to lock in pre-bid multiplier, and their additional bidAmount would not get taken into account since a more favourable multiplier is already locked.

Their ability to achieve a higher multiplier through the lack of accounting of the contribution of their tokens would invariably lead to another bidder being unable to redeem their tokens after the attacker has claimed their malicious earnings.

The auction becomes insolvent.

Impact

By closing the auction and bidding at precisely the auctionEndTime, an attacker can redeem the bidAmount for a higher number of tokens than deserved, whilst leaving honest bidders at a loss.

Tools Used

Manual Review

Recommendations

Correct the comparisons and don't allow any bidding or unbidding once the auction is closed:

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