Harsh Inky Peacock - RiskManager's incorrect decimal handling will cause catastrophic loss of user funds

[sherlock-admin2]

Harsh Inky Peacock

High

RiskManager's incorrect decimal handling will cause catastrophic loss of user funds

Summary

The convertToShares function's hardcoded assumption of 18 decimals for all assets will cause a catastrophic loss of funds for users depositing assets with fewer decimals (e.g., USDC, WBTC), as the function will calculate their share amount to be drastically lower than its actual value, effectively stealing the user's deposit.

Root Cause

In flexible-vaults/src/managers/RiskManager.sol:117, the convertToShares function converts an asset amount to a share amount using the formula Math.mulDiv(uint256(value), report.priceD18, 1 ether). The divisor 1 ether (10^18) is hardcoded. The function does not query the decimals() of the input asset. When a user deposits an asset like USDC which has 6 decimals, the value parameter will be scaled by 10^6. However, the calculation divides by 10^18. This results in the calculated shares being 10^12 times smaller than they should be. This incorrect share amount is then used to update vault and subvault balances, leading to a direct and massive loss for the depositor.

Internal Pre-conditions

1. A user deposits a token with fewer than 18 decimals (e.g., USDC with 6, WBTC with 8).
2. The RiskManager is used to calculate the shares for this deposit via functions like modifyVaultBalance or modifyPendingAssets.

External Pre-conditions

None.

Attack Path

This is a critical design flaw rather than a multi-step attack.

1. A user decides to deposit 1,000 USDC into a vault. The transaction value is 1,000 * 10^6.
2. The vault's logic calls RiskManager.modifyVaultBalance(USDC_ADDRESS, 1_000_000_000).
3. modifyVaultBalance calls convertToShares(USDC_ADDRESS, 1_000_000_000).
4. Inside convertToShares, the calculation is performed. Assuming the price of USDC is $1 (priceD18 = 1 * 10^18), the calculation becomes: (1_000 * 10^6 * 1 * 10^18) / 10^18 = 1_000 * 10^6 shares.
5. This seems correct, but the vault's shares are expected to be normalized to 18 decimals. The correct calculation should have been (1_000 * 10^6 * 1 * 10^18) / (1 * 10^6) = 1_000 * 10^18 shares.
6. The user is credited with 1,000 * 10^6 shares instead of 1,000 * 10^18 shares.
7. The user has effectively lost 99.9999999999% of their deposit value. When they later try to redeem their shares, they will only be able to withdraw a tiny fraction of their initial deposit.

Impact

Users depositing assets with fewer than 18 decimals will suffer a catastrophic, near-total loss of their deposited funds. For a USDC deposit, the user will lose (1 - 10^6 / 10^18) = 99.9999999999% of their funds instantly. This is a critical vulnerability that makes the protocol completely unusable with some of the most common stablecoins and assets in DeFi.

PoC

// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.25;

import "forge-std/Test.sol";
import "flexible-vaults/src/managers/RiskManager.sol";
import "flexible-vaults/test/mocks/MockERC20.sol";
import "flexible-vaults/src/interfaces/modules/IShareModule.sol";
import "flexible-vaults/src/interfaces/external/IOracle.sol";

// Mock ShareModule to return a mock Oracle
contract MockShareModule is IShareModule {
    IOracle public immutable oracle;
    constructor(IOracle _oracle) { oracle = _oracle; }
    // Implement other IShareModule functions as needed, returning default values
    function asset() external view returns (address) { return address(0); }
    function shareManager() external view returns (address) { return address(0); }
    function feeManager() external view returns (address) { return address(0); }
    function hasQueue(address) external view returns (bool) { return false; }
    function isPausedQueue(address) external view returns (bool) { return false; }
    function getLiquidAssets() external view returns (uint256) { return 0; }
    function callHook(uint256) external {}
}

// Mock Oracle to return a fixed price
contract MockOracle is IOracle {
    function getReport(address) external view returns (DetailedReport memory) {
        return DetailedReport({
            priceD18: 1 * 10**18, // Stablecoin price = $1
            timestamp: uint32(block.timestamp),
            isSuspicious: false
        });
    }
    // Implement other IOracle functions as needed
    function DOMAIN_SEPARATOR() external view returns (bytes32) { return bytes32(0); }
    function submitReports(Report[] calldata) external {}
    function acceptReport(address, uint224, uint32) external {}
}


contract RiskManager_RM_C1_Test is Test {
    RiskManager riskManager;
    MockShareModule mockShareModule;
    MockOracle mockOracle;
    MockERC20 usdc;
    address vault;

    function setUp() external {
        riskManager = new RiskManager("Mellow", 1);
        mockOracle = new MockOracle();
        vault = address(new MockShareModule(mockOracle));
        riskManager.setVault(vault);
        
        usdc = new MockERC20();
        usdc.setDecimals(6);
    }

    function test_RM_C1_IncorrectShareCalculation() external {
        // --- Simulation ---
        // A user deposits 1,000 USDC. The value is scaled by 6 decimals.
        int256 usdcDepositAmount = int256(1_000 * 10**6);

        // The RiskManager converts this amount to shares
        int256 calculatedShares = riskManager.convertToShares(address(usdc), usdcDepositAmount);

        // --- Verification ---
        // The expected number of shares should be 1,000 * 10**18
        int256 expectedShares = int256(1_000 * 10**18);

        // The actual calculated shares are 10^12 times smaller
        int256 actualShares = 1_000 * 10**6;
        
        assertEq(calculatedShares, actualShares, "Calculated shares are drastically wrong");
        assertTrue(calculatedShares < expectedShares, "User received a tiny fraction of correct shares");

        // This demonstrates a loss of (expected - actual) / expected
        // (1000e18 - 1000e6) / 1000e18 ~= 99.9999999999%
    }
}

Mitigation

No response