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

Incorrect accounting in `Treasury::_totalValue` leads to wrong total value held by treasury

kiteweb3

Summary

The Treasury contract implements a total value tracking system that adds all token amounts together regardless of their decimal places. This creates a fundamentally flawed accounting system where the _totalValue state variable aggregates incompatible numbers. The Treasury::getTotalValue should returns the total value held by treasury but it returns an incorrect number.

Vulnerability Details

function deposit(address token, uint256 amount) external override nonReentrant {
if (token == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
IERC20(token).transferFrom(msg.sender, address(this), amount);
_balances[token] += amount;
@> _totalValue += amount;
emit Deposited(token, amount);
}
​
function getTotalValue() external view override returns (uint256) {
@> return _totalValue;
}
​

Impact

The Treasury::deposit directly adds raw token amounts with different decimal places. In this way, the total value reported by getTotalValue is incorrect and the true protocol value/holdings can't be determined. Additionally, the _totalValue doesn't represent any meaningful economic value since doesn't account for token prices and doesn't use a common denomination (like USD).

Add Foundry to the project following this procedure

Create a file named Treasury.t.sol and copy/paste this:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;
​
import {Test, console2} from "forge-std/Test.sol";
import {Treasury} from "../contracts/core/collectors/Treasury.sol";
import {MockToken} from "../contracts/mocks/core/tokens/MockToken.sol";
import {MockUSDC} from "../contracts/mocks/core/tokens/MockUSDC.sol";
​
contract RTokenTest is Test {
Treasury public treasury;
MockToken public mockToken;
MockUSDC public mockUSDC;
​
address admin = makeAddr("admin");
​
function setUp() public {
treasury = new Treasury(admin);
mockToken = new MockToken("MOCK", "MOCK", 6);
mockUSDC = new MockUSDC(1000000 ether);
​
mockToken.mint(address(this), 1000 ether);
​
console2.log("treasury: ", address(treasury));
console2.log("mockToken: ", address(mockToken));
console2.log("mockUSDC: ", address(mockUSDC));
}
​
function test_IncorrectTotalValueAccounting() public {
assertEq(mockToken.decimals(), 6);
assertEq(mockUSDC.decimals(), 18);
​
mockToken.approve(address(treasury), type(uint256).max);
mockUSDC.approve(address(treasury), type(uint256).max);
treasury.deposit(address(mockToken), 1 * 1e6);
treasury.deposit(address(mockUSDC), 1 * 1e18);
​
assertNotEq(treasury.getTotalValue(), 2);
console2.log("_totalValue: ", treasury.getTotalValue());
}

Run forge test --match-test test_IncorrectTotalValueAccounting -vv

Logs:
treasury: 0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
mockToken: 0x2e234DAe75C793f67A35089C9d99245E1C58470b
mockUSDC: 0xF62849F9A0B5Bf2913b396098F7c7019b51A820a
_totalValue: 1000000000001000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.36ms (441.54µs CPU time)

The test shows that the _totalValue is a meaningless number.

Tools Used

Manual review

Recommendations

Add decimal normalization that converts all token amounts to a standard decimal format.

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 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.