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

Loss of rewards and assets occurs in the FjordAuction contract's claimTokens() function due to improper handling of auction token decimals

Description

Solidity rounds down the result of an integer division, and because of that, it is always recommended to multiply before dividing to avoid that precision loss.
In the case of a prior division over multiplication, the final result may face serious precision loss as the first answer would face truncated precision and
then multiplied to another integer.
The problem arises in the FjordAuction's auctionEnd() function which is responsible for calculating the multiplier variable:

function auctionEnd() external {
...
multiplier = totalTokens.mul(PRECISION_18).div(totalBids); //@audit Not checking the precision of tokens may result is zero
// Burn the FjordPoints held by the contract
uint256 pointsToBurn = fjordPoints.balanceOf(address(this));
fjordPoints.burn(pointsToBurn);
}

As one can see here, the multiplier is calculated using the multiplication of totalTokens and 1e18 and then divided by the totalBids.
This by default is not a problem, however, considering the fact that when claiming the token, this variable is multiplied by the userBids,
we may face significant precision loss (especially when the auction token's decimal is pretty low e.g. WBTC with 8 decimals):

function claimTokens() external {
...
uint256 claimable = userBids.mul(multiplier).div(PRECISION_18);
bids[msg.sender] = 0;
auctionToken.transfer(msg.sender, claimable);
emit TokensClaimed(msg.sender, claimable);
}

Thus, in the case of high bids, and low token decimals, bidders couldn't be able to call claimTokens() and lose both their accumulated auction token rewards as well as their initial Fjord points investment.

Impact

  1. Loss of auction tokens and Fjord Points due to the precision loss in case of low auction token decimal

  2. The sum of individual claimables may not be equal the initial considered rewards

Proof of Concept

To illustrate the issue, we assume the auction token to be WBTC with a decimal of 8, and the total rewards of 100 WBTC.
In this specific test, we take the preceding steps:

  1. An auction is started with 100 WBTC by the owner

  2. Users bid ~ 4.999e28 points on the auction

  3. Alice bids 8.8246e24 (to make the total bids become equal to 5e28)

  4. After the auctionEnd() is called, Alice will lose her rewards as well as point investments

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
import "../src/FjordStaking.sol";
import { FjordPoints } from "../src/FjordPoints.sol";
import { Test } from "forge-std/Test.sol";
import { MockERC20 } from "solmate/test/utils/mocks/MockERC20.sol";
import {FjordAuction} from "../src/FjordAuction.sol";
contract FjordTest is Test {
FjordAuction auction;
FjordStaking fjordStaking;
MockERC20 token;
MockERC20 wbtc;
FjordPoints points;
address internal constant SABLIER_ADDRESS = address(0xB10daee1FCF62243aE27776D7a92D39dC8740f95);
address minter = makeAddr("minter");
address newMinter = makeAddr("new_minter");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address authorizedSender = address(this);
function setUp() public {
points = new FjordPoints();
token = new MockERC20("Fjord", "FJO", 18);
fjordStaking =
new FjordStaking(address(token), address(this), SABLIER_ADDRESS, authorizedSender, address(points));
points.setStakingContract(address(fjordStaking));
wbtc = new MockERC20("Wrapped Bitcoin", "WBTC", 8);
auction = new FjordAuction(address(points), address(wbtc), 7 days, 10e9);
}
function testPrecisionLoss() public {
deal(address(points), address(this), 1e30);
assertEq(points.balanceOf(address(this)), 1e30);
points.approve(address(auction), type(uint256).max);
auction.bid(49991.1754e24);
vm.warp(block.timestamp + 2 days);
deal(address(points), alice, 1e25);
assertEq(points.balanceOf(address(alice)), 1e25);
vm.startPrank(alice);
points.approve(address(auction), type(uint256).max);
auction.bid(8.8246e24);
vm.stopPrank();
vm.warp(block.timestamp + 7 days);
auction.auctionEnd();
vm.prank(alice);
auction.claimTokens();
console.log("Alice's auction token balance after claiming is: ", token.balanceOf(alice));
// Manual calculation of rewards based on the bids of Alice and total bids of the auction
uint actualReward = (8.8246e24 * 10e9 * 1e18) / (1e18 * 5e28);
console.log("Alice's actual auction token balance should be: ", actualReward);
}
}

The result is:

Ran 1 test for test/FjordTest.t.sol:FjordTest
[PASS] testPrecisionLoss() (gas: 537906)
Logs:
Alice's auction token balance after claiming is: 0
Alice's actual auction token balance should be: 1764920
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.29ms (2.13ms CPU time)

This test shows that for the case of totalBids = 5e28, and Alice's userBids = 8.8246e24, WBTC's totalTokens = 10e9:

Expected rewards for Alice: 1764920 (~ $1058 with WBTC price equal to $60k)
Actual rewards for Alice: 0

Tools Used

Manual Review

Recommendations

Consider selecting a proper precision multiplier considering the auction tokens decimal and also prioritizing the multiplication over division.

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Low decimal tokens or super small bids can lead to 0 claims

Appeal created

matin Auditor
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Low decimal tokens or super small bids can lead to 0 claims

Support

FAQs

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