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

Users can bid after auction has ended in FjordAuction.sol. This breaks internal accounting for how many auctionTokens should be sent to a user.

sungyuk1

Summary

The FjordAuction.sol contract has external functions for bid(), unbid(), and auctionEnd(). Both the unbid() and bid() functions will revert if the current block.timestamp is larger than auctionEndTime as shown below. However, these functions will not revert if the current block.timestamp is equal to the auctionEndTime. In such a case, a user can place a bid during the same block that the auctionEnd() function is called. This will result in user transferring fjordPoints to the contract which increases their balance in the bids mapping, while not diluting the multiplier calculated in auctionEnd().

Vulnerability Details

Below in the bid function, notice how the bid function will only revert if block.timestamp is larger than auctionEndTime. If block.timestamp is equal to auctionEndTime the transaction does not revert.

/**
* @notice Places a bid in the auction.
* @param amount The amount of FjordPoints to bid.
*/
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);
}

Notice how the auctionEnd() function reverts only if the current block.timestamp is less than auctionEndTime.

/**
* @notice Ends the auction and calculates claimable tokens for each bidder based on their bid proportion.
*/
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);
}

This means that both the bid() and auctionEnd() functions can be called during the same block, when block.timestamp is equal to auctionEndTime. In such scenarios, if a bid() function call is processed after the auctionEnd() function call, the user will increase their balance in the bids mapping while not decreasing the multiplier calculated in auctionEnd().

Impact

The multiplier state variable is used to calculate how many tokens a user can claim considering the amount they deposited.

function claimTokens() external {
if (!ended) {
revert AuctionNotYetEnded();
}
​
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoTokensToClaim();
}
​
uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
bids[msg.sender] = 0;
​
auctionToken.transfer(msg.sender, claimable);
emit TokensClaimed(msg.sender, claimable);
}

Since this vulnerability will cause the multiplier to be inflated, that means that each call to claimTokens() will give out more tokens than it should proportionally to how much users have deposited. This will lead to the contract running out of auctionTokens and the users who call claimTokens() late will not be be able to get anything.

Tools Used

Manual Review

Recommendations

Super simple fix, just revert on bid/unbid function calls when block.timestamp is greater than or equal to auctionEndTime, rather than just greater

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