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

Auction end time incorrect checking

pontifex

Summary

Inconsistency of auction time checking in bid and auctionEnd functions let bidding when the auction is ended. This can brake claim tokens functionality for a group of users.

Vulnerability Details

Due to the bid function check the auction is considered ended when block.timestamp > auctionEndTime:

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

But it is possible to execute the auctionEnd function when 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);
}

This way a malicious user can invoke the auctionEnd function, then bid with sufficient amount of FjordPoints (up to totalBids), then claimTokens. Since this last bid was not included in the multiplier the token distribution will be incorrect and the claimTokens will throw insufficient balance error for all or a part of users.

Impact

Asset losses, permanent asset locking

Tools used

Manual Review

Recommendations

Consider fixing the inconsistency in checks:

function auctionEnd() external {
- if (block.timestamp < auctionEndTime) {
+ if (block.timestamp <= auctionEndTime) {
revert AuctionNotYetEnded();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

If no bids are placed during the auction, the `auctionToken` will be permanently locked within the `AuctionFactory`

An auction with 0 bids will get the `totalTokens` stuck inside the contract. Impact: High - Tokens are forever lost Likelihood - Low - Super small chances of happening, but not impossible

Appeal created

demorextess Judge
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
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.