Fjord Token Staking

Relevant GitHub Links

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L144

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L182

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L197

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L217

Summary

An attacker can exploit the auction contract by ending the auction and placing a bid, and then claiming in the same block when block.timestamp == auctionEndTime. This manipulation allows the attacker to bypass the correct calculation of the multiplier for token distribution, resulting in them unfairly claiming a large share of tokens at the expense of other users who participated fairly.

Vulnerability Details

The vulnerability arises due to a timing issue when block.timestamp == auctionEndTime. An attacker can call the auctionEnd() function and immediately follow it with a bid() in the same block. Because the multiplier calculation in auctionEnd() is based on the totalBids at the moment of execution, it doesn't include the attacker's subsequent bid. Then, by calling claimTokens(), the attacker can claim a disproportionate amount of tokens based on their unaccounted bid, effectively stealing tokens that should have been distributed among other users.

Impact

An attacker can exploit the auction contract when block.timestamp == auctionEndTime by:

1. Call auctionEnd(): The attacker ends the auction, calculating the multiplier as totalTokens * 1e18 / totalBids using the totalBids before the attacker’s bid is added. This results in a bigger multiplier since the attacker’s bid, which increases totalBids, is not accounted for yet.

2. Call bid(): The attacker places a large bid immediately after auctionEnd(), increasing totalBids but not updating the previously calculated multiplier.

3. Call claimTokens(): The attacker claims tokens using the outdated multiplier from the auctionEnd() calculation. Since the multiplier was based on a lower totalBids, the claimable amount becomes disproportionately high, calculated as userBids * multiplier / 1e18.

This allows the attacker to claim a larger share of tokens than they are entitled to, as the multiplier is inflated due to the outdated totalBids used in the calculation. The discrepancy between totalBids at the time of auction end and the actual totalBids after the additional bid results in an unfair advantage, enabling the attacker to steal tokens from other users.

Tools Used

Manual review

Recommendations

To prevent this exploit, modify the check in the auctionEnd() function to ensure that it cannot be called exactly at the auctionEndTime. The check should be:

if (block.timestamp < =

auctionEndTime) { revert AuctionNotYetEnded(); }

This change will ensure that the auction can only be ended after the official end time, preventing the execution of bid() and auctionEnd() in the same block when block.timestamp == auctionEndTime, thereby mitigating the exploit.