DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: low
Invalid

Precision loss in `FjordAuction::claimTokens` and `FjordAuction::auctionEnd` causes tokens to be permanently locked in the `FjordAuction` contract and bidders not getting their expected amount of tokens

Summary

The PRECISION_18 variable is used in FjordAuction::auctionEnd and FjordAuction::claimTokens to maintain precision during calculation of the distribution of auction tokens. However when the total amount of tokens or the bids placed are too high comapred to PRECISION_18, it causes a precision loss. It allows high value bids or tokens with low decimals to cause a precision loss resulting in bidders not getting the expected amount of tokens and small amount of tokens being stuck in the contract, which cannot be claimed or retrieved, leading to permanent loss of those tokens.

Vulnerability Details

Relevant links

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

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

The FjordAuction uses the PRECISION_18 variable to maintain precision during distribution of auction token among bidders. This value is used in FjordAuction::auctionEnd to calculate the multiplier for FjordAuction::claimTokens function and it is also used in FjordAuction::claimTokens to calculate the exact no of tokens a bidder should receive. If PRECISION_18 variable is too low compared to the total amount of tokens or the bids placed, it causes a rounding error or a precision loss, especially with high value bids or tokens with low decimals.

Impact

Small amount of tokens are stuck in the contract, which cannot be claimed or retrieved. It also means that bidders are not getting the expected amount of tokens as they are lost due to precision lost.

Proof of Concept

The impact is demonstrated with the following test, which can be executed with forge test --mt testTokensAreNotLostDuringCalculation.

// 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";
import {ERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
import {ERC20Burnable} from "lib/openzeppelin-contracts/contracts/token/ERC20/extensions/ERC20Burnable.sol";
contract TestAuction is Test {
using SafeMath for uint256;
FjordAuction public auction;
ERC20BurnableMock public fjordPoints;
ERC20AuctionToken public auctionToken;
address public owner = address(0x1);
uint256 public biddingTime = 1 weeks;
uint256 public totalTokens = 100e6;
function setUp() public {
fjordPoints = new ERC20BurnableMock("FjordPoints", "fjoPTS");
auctionToken = new ERC20AuctionToken("AuctionToken", "AUCT");
auction = new FjordAuction(
address(fjordPoints),
address(auctionToken),
biddingTime,
totalTokens
);
deal(address(auctionToken), address(auction), totalTokens);
}
function testTokensAreNotLostDuringCalculation() public {
address bidder = address(0x2);
address bidder2 = address(0x3);
uint256 bidAmount = 1000000 ether;
uint256 bidAmount2 = 1 ether;
deal(address(fjordPoints), bidder, bidAmount);
deal(address(fjordPoints), bidder2, bidAmount2);
vm.startPrank(bidder);
fjordPoints.approve(address(auction), bidAmount);
auction.bid(bidAmount);
vm.stopPrank();
vm.startPrank(bidder2);
fjordPoints.approve(address(auction), bidAmount2);
auction.bid(bidAmount2);
vm.stopPrank();
skip(biddingTime);
auction.auctionEnd();
vm.prank(bidder);
auction.claimTokens();
vm.prank(bidder2);
auction.claimTokens();
console2.log(auctionToken.balanceOf(address(auction))); // output: 999901
console2.log(auctionToken.balanceOf(bidder)); // output: 99000000
console2.log(auctionToken.balanceOf(bidder2)); // output: 99
assertEq(auctionToken.balanceOf(address(auction)), 1);
assertEq(auctionToken.balanceOf(address(bidder)), 99999900); // decimal value: 99.9999000001
assertEq(auctionToken.balanceOf(address(bidder2)), 99); // decimal value: 0.0000999999000001
}
}
contract ERC20AuctionToken is ERC20, ERC20Burnable {
constructor(
string memory name_,
string memory symbol_
) ERC20(name_, symbol_) {}
function decimals() public view virtual override returns (uint8) {
return 8;
}
}

This test confirms that tokens are being stuck in the contract and the bidders are not getting the expected amount of tokens if the bidAmount is high and the decimal of the auction token is low.

Tools Used

Manual Review, Foundry

Recommendations

Increase the value of PRECISION_18 to 1e28 or higher to ensure that the new precision variable is always larger than any number leading to accurate distribution. Also rename PRECISION_18 to reflect the updated value (e,g, PRECISION_28).

After adding this change, you can rerun the poc test provided earlier to verify that the bidders are getting the expected amount of tokens and tokens are not stuck inside the contract.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

FjordAuction doesn't handle the dust remained after everyone claimed

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.