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

Potential Overlap in multiple function of FjordAuction.sol may lead to inconsistent results

oxwhite

Summary

There are 3 functions in FjordAuction.sol contract that have potential overlap in the last second of an auction, which might lead to unintented results.

Vulnerability Details

FjordAuction.sol deals with bidding, unbidding , ending and a bunch of other functionalities. Let's look at 3 functions closely, which are bid(), unbid(), auctionEnd() .With the current implementation, if block.timestamp == auctionEndTime the following operations can all occur:

Bids Can Be Placed:

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

Bids Can Be Withdrawn (Unbid):

function unbid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
​
uint256 userBids = bids[msg.sender];
if (userBids == 0) {
revert NoBidsToWithdraw();
}
if (amount > userBids) {
revert InvalidUnbidAmount();
}
​
bids[msg.sender] = bids[msg.sender].sub(amount);
totalBids = totalBids.sub(amount);
fjordPoints.transfer(msg.sender, amount);
emit BidWithdrawn(msg.sender, amount);
}

Auction Can Be Ended:

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

Unclear timing boundry among the functions may lead to inconsistencies in the final total of bids and potential issues for users. Imagine a scenario that bidder1(Alice) , bidder2(bob) and bidder3(Eve) calls bid(), unbid(), auctionEnd() respectively . Now depending on the order of execution of the functions different outcomes will appear. As an example, if the call made to auctionEnd() executed before Alice's call, then this may prevent Alice from accessing auction tokens. If there was a more clear timing check set in auctionEnd(), such as:

if (block.timestamp <= auctionEndTime) {
revert AuctionNotYetEnded();
}

then Eve would not able to call the function and Alice could potentially get her tokens based on the bid she made. Contract's state would be affected as well. The state variable multiplier is calculated based on totalBids:

function auctionEnd() external {
//SNIP
​
multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
//SNIP
}

The multiplier is later used in calculating claimable amounts.

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

In short, It is clear that due to the code implementation and potential last bids, users' claimable amount can be manipulated.

Impact

Due to the uncertainty in final bids, Bidders may intentionally/uintentionally face undesired/desired outcomes.

Tools Used

Manual Review, Vs Code

Recommendations

Remove the uncertainty in final bids

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.