Core Contracts

Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Submission Details
Severity: medium
Valid

Incorrect Handling of Decimals During Treasury Total Value Calculation

Incorrect Total Value Calculation

Description

The Treasury contract calculates the total value of funds held by summing the raw amounts of different tokens without considering their decimal places or market values. This can lead to misleading financial reporting and an inaccurate representation of the treasury's actual value.

Key Issues

  1. Raw Sum Calculation:

    • The total value is calculated as a simple sum of token amounts, ignoring the fact that different tokens may have different decimal places. For example, 100 DAI (with 18 decimals) and 100 USDC (with 6 decimals) would be summed without adjusting for their actual value.

  2. Misleading Financial Reporting:

    • The total value metric may give a false impression of the treasury's financial health. Users may believe that the treasury has more value than it actually does, leading to poor decision-making.

  3. Lack of Market Value Consideration:

    • The current implementation does not account for the market value of tokens, which can fluctuate significantly. This means that the reported total value may not reflect the true economic value of the treasury's holdings.

Example Scenario

  1. Initial Setup:

    • The treasury holds 100 DAI (18 decimals) and 100 USDC (6 decimals).

    • The total value is calculated as:

      • Total Value = 100 DAI + 100 USDC = 200 (raw sum).

  2. Problem Arises:

    • The actual value of the treasury is:

      • 100 DAI = $100 (assuming 1 DAI = $1)

      • 100 USDC = $100 (assuming 1 USDC = $1)

    • The total value should be reported as $200, but the raw sum does not account for the different decimal places, leading to potential confusion.

Consequences

  • Inaccurate Financial Reporting: Users may make decisions based on misleading information about the treasury's value.

  • Potential Legal Issues: If the treasury is required to report its financial status, inaccurate reporting could lead to legal ramifications.

  • Loss of Trust: Users may lose trust in the protocol if they discover discrepancies between reported values and actual values.

POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import {Treasury} from "../contracts/core/collectors/Treasury.sol";
import {MockERC20} from "../test/mocks/MockERC20.sol";
contract TreasuryTest is Test {
Treasury public treasury;
MockERC20 public token;
MockERC20 public token2;
address admin = makeAddr("admin");
address manager = makeAddr("manager");
address allocator = makeAddr("allocator");
address allocator2 = makeAddr("allocator2");
address user = makeAddr("user");
address recipient = makeAddr("recipient");
uint256 constant INITIAL_BALANCE = 1000e18;
event Deposited(address indexed token, uint256 amount);
event Withdrawn(address indexed token, uint256 amount, address indexed recipient);
event FundsAllocated(address indexed recipient, uint256 amount);
function setUp() public {
console.log("==== Setting Up Test Environment ====");
// Deploy contracts
treasury = new Treasury(admin);
token = new MockERC20("Test Token", "TEST");
token2 = new MockERC20("Test Token 2", "TEST2");
token2.setDecimals(6);
console.log("Deployed Treasury & Token contracts");
console.log("Admin:", admin);
console.log("Manager:", manager);
console.log("Allocator1:", allocator);
console.log("Allocator2:", allocator2);
console.log("User:", user);
console.log("Recipient:", recipient);
// Setup roles
vm.startPrank(admin);
treasury.grantRole(treasury.MANAGER_ROLE(), manager);
treasury.grantRole(treasury.ALLOCATOR_ROLE(), allocator);
treasury.grantRole(treasury.ALLOCATOR_ROLE(), allocator2);
vm.stopPrank();
console.log("Roles granted successfully");
// Setup initial token balances
token.mint(user, INITIAL_BALANCE);
token2.mint(user, 1000e6);
console.log("Initial token balances minted for user");
}
// Test: Incorrect Total Value Calculation due to mixing decimals
function testIncorrectTotalValueCalculation() public {
console.log("==== Starting testIncorrectTotalValueCalculation ====");
vm.startPrank(user);
console.log("User:", user, "is about to approve tokens");
// Approve treasury for depositing tokens
token.approve(address(treasury), 100e18);
console.log("Approved 100e18 tokens for Treasury on token with 18 decimals");
token2.approve(address(treasury), 100e6);
console.log("Approved 100e6 tokens for Treasury on token2 with 6 decimals");
// Log balances before deposit
uint256 balanceTokenBefore = token.balanceOf(user);
uint256 balanceToken2Before = token2.balanceOf(user);
console.log("User token balance before deposit (18 decimals):", balanceTokenBefore);
console.log("User token2 balance before deposit (6 decimals):", balanceToken2Before);
// Deposit tokens with different decimals
console.log("Depositing token (18 decimals)...");
treasury.deposit(address(token), 100e18); // 100 tokens with 18 decimals
console.log("Depositing token2 (6 decimals)...");
treasury.deposit(address(token2), 100e6); // 100 tokens with 6 decimals
vm.stopPrank();
console.log("Deposits complete");
// Log balances after deposit (if needed, assuming tokens are ERC20 compliant)
uint256 balanceTokenAfter = token.balanceOf(user);
uint256 balanceToken2After = token2.balanceOf(user);
console.log("User token balance after deposit (18 decimals):", balanceTokenAfter);
console.log("User token2 balance after deposit (6 decimals):", balanceToken2After);
// Query total treasury value
uint256 tot = treasury.getTotalValue();
console.log("Total Value stored in treasury:", tot);
// Final assertion check
uint256 expected = 100e18 + 100e6;
console.log("Expected total value:", expected);
assertEq(tot, expected, "Total value does not match the expected value");
console.log("==== testIncorrectTotalValueCalculation completed successfully ====");
}
}

Recommendations

  1. Implement Decimal Normalization:

    • Adjust the total value calculation to account for the decimal places of each token. This can be done by converting all token amounts to a common base (e.g., using the smallest unit of each token).

  2. Consider Market Values:

    • Integrate a price oracle to provide real-time market values for each token held in the treasury. This would allow for a more accurate representation of the treasury's total value.

  3. Clear Documentation:

    • Document the limitations of the total value calculation clearly in the code and user interfaces to prevent misunderstandings.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Treasury::deposit increments _totalValue regardless of the token, be it malicious, different decimals, FoT etc.

Support

FAQs

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