Skip to content

Latest commit

 

History

History
538 lines (418 loc) · 23.1 KB

File metadata and controls

538 lines (418 loc) · 23.1 KB

Energetic Opaque Elephant

Medium

Denial-of-Service Vulnerability due to Unbounded Loop in __WeightedIndex_init

Summary

The __WeightedIndex_init function in the WeightedIndex contract contains a loop that iterates up to 255 times, processing each token added to the index. This unbounded loop, combined with gas-intensive operations within the loop, creates a potential Denial-of-Service (DoS) vulnerability. An attacker can exploit this by submitting a transaction with a large number of tokens (up to the loop limit), causing the transaction to consume excessive gas and potentially exceeding the block gas limit. This can prevent legitimate users from interacting with the contract and disrupt its intended functionality.

Root Cause

The primary root cause is the unbounded nature of the loop in WeightedIndex::__WeightedIndex_init While the number of iterations is limited by the uint8 loop counter _i (max 255), the gas cost per iteration, especially with operations like the q1 calculation and decimals() calls, is significant. This allows an attacker to manipulate the gas cost to create a DoS condition.

Internal Pre-conditions

  • The WeightedIndex contract is deployed and operational.
  • The __WeightedIndex_init function is accessible (directly or via a wrapper function).
  • The contract relies on the indexTokens array and _fundTokenIdx mapping, which are populated within the loop.
  • The contract uses a FixedPoint96 library and interacts with an IERC20Metadata interface.

External Pre-conditions

  • The attacker has control over the input parameters to the __WeightedIndex_init function, specifically the _tokens and _weights arrays.
  • The attacker has sufficient ETH to pay for the gas cost of the attack transaction (though the cost can be made high enough to be a griefing attack, even if it doesn't exceed the block limit).

Attack Path

  1. The attacker crafts a transaction that calls the __WeightedIndex_init function (or the publicInit wrapper) with a large number of tokens (up to 255).
  2. The _tokens array contains valid token addresses (that are deployed contracts) and the _weights array contains corresponding weights.
  3. The transaction is submitted to the Ethereum network.
  4. The loop in __WeightedIndex_init iterates through the provided tokens, performing gas-intensive operations for each token.
  5. The total gas cost of the transaction increases significantly with the number of tokens.
  6. Scenario 1 (Block Gas Limit): If the gas cost exceeds the block gas limit, the transaction is reverted. This prevents the attacker from successfully executing the attack but also potentially prevents legitimate transactions from being included in the block.
  7. Scenario 2 (Gas Griefing): If the gas cost is high but below the block gas limit, the transaction is included in the block. This makes it very expensive for anyone else to interact with the contract. Even if they can afford the gas, the contract owner will be griefed by the high gas cost of any subsequent transaction.

Impact

  • Denial of Service: Legitimate users are unable to interact with the WeightedIndex contract.
  • Gas Griefing: The cost of interacting with the contract becomes prohibitively high, effectively making it unusable for legitimate users.
  • Reputational Damage: The contract's reputation is harmed, and users may lose trust in the system.
  • Financial Loss: Users may be unable to access or manage their assets within the index.

PoC

Add the provided test file (containing the test_DoS_attack_large_number_of_tokens and test_DoS_mitigation functions) to your test folder. This file demonstrates the potential Denial-of-Service vulnerability and the implemented mitigation.

import "@openzeppelin/contracts/interfaces/IERC20.sol";
import {console2} from "forge-std/Test.sol";
import {PEAS} from "../contracts/PEAS.sol";
import {RewardsWhitelist} from "../contracts/RewardsWhitelist.sol";
import {V3TwapUtilities} from "../contracts/twaputils/V3TwapUtilities.sol";
import {UniswapDexAdapter} from "../contracts/dex/UniswapDexAdapter.sol";
import {IDecentralizedIndex} from "../contracts/interfaces/IDecentralizedIndex.sol";
import {IStakingPoolToken} from "../contracts/interfaces/IStakingPoolToken.sol";
import {WeightedIndex} from "../contracts/WeightedIndex.sol";
import {MockFlashMintRecipient} from "./mocks/MockFlashMintRecipient.sol";
import {PodHelperTest} from "./helpers/PodHelper.t.sol";
import "forge-std/console.sol";
import {MockERC20, MockUniswapV2Router, MockPEAS, MockUniswapV2Pair, MockUniswapV2Factory} from "./MockERC20.sol";
import {TestWeightedIndex} from "./TestWeightedIndex.t.sol";
import "@openzeppelin/contracts/utils/Strings.sol";

contract WeightedIndexTest is PodHelperTest {
    //PEAS public peas;
    RewardsWhitelist public rewardsWhitelist;
    V3TwapUtilities public twapUtils;
    UniswapDexAdapter public dexAdapter;
    WeightedIndex public pod;
    MockFlashMintRecipient public flashMintRecipient;

    MockERC20 public dai; // Use MockERC20 for DAI
    MockUniswapV2Router public mockV2Router;
    MockERC20 public mockWeth;
    MockPEAS public peas; // Use MockPEAS
    MockUniswapV2Factory public mockV2Factory;
    MockUniswapV2Pair public mockPair;
    TestWeightedIndex public podLarge;
    
    
    address public mockPairAddress;
    address public mockV2FactoryAddress;
    //address dummyFactory = address(0x123);
    address public peasAddress;
    address public mockDAI; // Address of the deployed mock DAI
    //address public dai = 0x6B175474E89094C44Da98b954EedeAC495271d0F;
    uint256 public bondAmt = 1e18;
    uint16 fee = 100;
    uint256 public bondAmtAfterFee = bondAmt - (bondAmt * fee) / 10000;
    uint256 public feeAmtOnly1 = (bondAmt * fee) / 10000;
    uint256 public feeAmtOnly2 = (bondAmtAfterFee * fee) / 10000;

    // Test users
    address public alice = address(0x1);
    address public bob = address(0x2);
    address public carol = address(0x3);

    event FlashMint(address indexed executor, address indexed recipient, uint256 amount);

    event AddLiquidity(address indexed user, uint256 idxLPTokens, uint256 pairedLPTokens);

    event RemoveLiquidity(address indexed user, uint256 lpTokens);

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

        // 1. Deploy Mock ERC20s FIRST
        dai = new MockERC20("MockDAI", "mDAI", 18);
        mockDAI = address(dai);
        mockWeth = new MockERC20("Wrapped Ether", "WETH", 18);
        podLarge = new TestWeightedIndex();

        // 2. Deploy Mock Factory
        mockV2Factory = new MockUniswapV2Factory();
        mockV2FactoryAddress = address(mockV2Factory);

        // 3. Deploy Mock Router (using the factory address!)
        mockV2Router = new MockUniswapV2Router(address(mockWeth), mockV2FactoryAddress);


        // 4. Deploy Mock PEAS
        peas = new MockPEAS("PEAS", "PEAS", 18);
        peasAddress = address(peas);

        // 5. Create and register the Mock Pair
        mockPair = new MockUniswapV2Pair(address(dai), address(mockWeth));
        mockPairAddress = address(mockPair);
        mockV2Factory.setPair(address(dai), address(mockWeth), mockPairAddress); // VERY IMPORTANT!

        // 6. Initialize the DEX Adapter (using the router)
        dexAdapter = new UniswapDexAdapter(
            twapUtils,
            address(mockV2Router),
            0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45, // Uniswap SwapRouter02
            false
        );
        
        IDecentralizedIndex.Config memory _c;
        IDecentralizedIndex.Fees memory _f;
        _f.bond = fee;
        _f.debond = fee;
        address[] memory _t = new address[](1);
        _t[0] = address(peas);
        uint256[] memory _w = new uint256[](1);
        _w[0] = 100;
        address _pod = _createPod(
            "Test",
            "pTEST",
            _c,
            _f,
            _t,
            _w,
            address(0),
            false,
            abi.encode(
                mockDAI,
                //dai,
                peasAddress,
                //address(peas),
                mockDAI,
                //0x6B175474E89094C44Da98b954EedeAC495271d0F,
                0x7d544DD34ABbE24C8832db27820Ff53C151e949b,
                rewardsWhitelist,
                0x024ff47D552cB222b265D68C7aeB26E586D5229D,
                dexAdapter
            )
        );
        pod = WeightedIndex(payable(_pod));

        flashMintRecipient = new MockFlashMintRecipient();

        // Initial token setup for test users
        deal(address(peas), address(this), bondAmt * 100);
        deal(address(peas), alice, bondAmt * 100);
        deal(address(peas), bob, bondAmt * 100);
        deal(address(peas), carol, bondAmt * 100);
        deal(mockDAI, address(this), 5 * 10e18);

        // Approve tokens for all test users
        vm.startPrank(alice);
        peas.approve(address(pod), type(uint256).max);
        vm.stopPrank();

        vm.startPrank(bob);
        peas.approve(address(pod), type(uint256).max);
        vm.stopPrank();

        vm.startPrank(carol);
        peas.approve(address(pod), type(uint256).max);
        vm.stopPrank();
    }

    function test_WeightedIndex_inits() public {
        IDecentralizedIndex.Config memory _config;
        address[] memory _tokens = new address[](3);
        uint256[] memory _weights = new uint256[](3);
        bytes memory _immutables;

        _tokens[0] = address(dai);
        _tokens[1] = address(peas);
        _tokens[2] = address(mockWeth);

        _weights[0] = 50;
        _weights[1] = 30;
        _weights[2] = 20;

        _immutables = abi.encode(
            address(dai),
            address(peas),
            address(dai),
            0x7d544DD34ABbE24C8832db27820Ff53C151e949b,
            rewardsWhitelist,
            0x024ff47D552cB222b265D68C7aeB26E586D5229D,
            dexAdapter
        );

        podLarge.publicInit(_config, _tokens, _weights, _immutables); // Use publicInit

        assertEq(podLarge.indexTokenCount(), 3, "Incorrect token count");
        // assertEq(pod.indexTokens(0).token, address(dai), "Incorrect token at index 0");
        // assertEq(podLarge.indexTokens(1).token, address(peas), "Incorrect token at index 1");
        // assertEq(podLarge.indexTokens(2).token, address(mockWeth), "Incorrect token at index 2");

        assertEq(podLarge.getFundTokenIdx(address(dai)), 0, "Incorrect index for DAI"); // Use getFundTokenIdx
        assertEq(podLarge.getFundTokenIdx(address(peas)), 1, "Incorrect index for PEAS"); // Use getFundTokenIdx
        assertEq(podLarge.getFundTokenIdx(address(mockWeth)), 2, "Incorrect index for WETH"); // Use getFundTokenIdx
        //assertEq(podLarge.totalWeights(), 100, "incorrect total weight");

        // Add more assertions as needed (e.g., weights, q1 values, blacklist)
    }

    function test_DoS_attack_large_number_of_tokens() public {
        IDecentralizedIndex.Config memory _config;
        uint256 numTokens = 255; // Max allowed by uint8 loop counter
        address[] memory _tokens = new address[](numTokens);
        uint256[] memory _weights = new uint256[](numTokens);
        bytes memory _immutables;

        for (uint256 i = 0; i < numTokens; i++) {
            MockERC20 mockToken = new MockERC20(
                string(abi.encodePacked("MockToken", Strings.toString(i))),
                string(abi.encodePacked("MT", Strings.toString(i))),
                18
            );
            _tokens[i] = address(mockToken);
            _weights[i] = 100;

            mockToken.mint(address(pod), 1000000 ether);
            mockToken.mint(address(this), 1000000 ether); // For any testing interactions
        }

        _immutables = abi.encode(
            address(dai),
            address(peas),
            address(dai),
            0x7d544DD34ABbE24C8832db27820Ff53C151e949b,
            rewardsWhitelist,
            0x024ff47D552cB222b265D68C7aeB26E586D5229D,
            dexAdapter
        );

        // Try to initialize with the maximum number of tokens
        // This test will demonstrate the potential for DoS due to gas griefing
        // or exceeding the block gas limit.  The exact outcome depends on 
        // the gas cost of the operations within the loop.

        // Option 1: Expect a revert due to exceeding gas limit (if it does)
        // vm.expectRevert();  // Uncomment if you expect the transaction to revert

        // Option 2: Check gas used (more precise, but test might need gas limit increase)
        uint256 gasBefore = gasleft();
        podLarge.publicInit(_config, _tokens, _weights, _immutables);
        uint256 gasUsed = gasBefore - gasleft();
        console.log("Gas used:", gasUsed);

        // You can set an expected gas usage range. This needs careful tuning!
        // uint256 expectedGasMin = ...; // Set a reasonable min gas usage
        // uint256 expectedGasMax = ...; // Set a reasonable max gas usage
        // assertGe(gasUsed, expectedGasMin, "Gas used is less than expected");
        // assertLe(gasUsed, expectedGasMax, "Gas used is greater than expected");

        // The gas usage will depend on the operations in __WeightedIndex_init
        // and the state of your mock contracts.  It's best to log and analyze
        // the gas usage first to determine a reasonable expected range.

        // Add assertions to check the state after (if the tx does not revert)
        assertEq(podLarge.indexTokenCount(), numTokens, "Incorrect token count");
        for (uint256 i = 0; i < numTokens; i++) {
           assertEq(podLarge.getFundTokenIdx(_tokens[i]), i, "Incorrect index for token");
        }
        assertEq(podLarge.totalWeights(), numTokens * 100, "incorrect total weight");

    }

// A passing `test_DoS_attack_large_number_of_tokens` test (without `expectRevert()`) 
//is a sign of a vulnerability, not a sign of safety.  You must implement mitigations 
//to protect your contract from gas griefing and potential DoS attacks.  
//Don't ignore this just because the test "passes" in its current form


function test_DoS_mitigation() public {
        IDecentralizedIndex.Config memory _config;
        uint256 numTokens = 256; // One more than the limit
        address[] memory _tokens = new address[](numTokens);
        uint256[] memory _weights = new uint256[](numTokens);
        bytes memory _immutables;

        for (uint256 i = 0; i < numTokens; i++) {
            MockERC20 mockToken = new MockERC20(
                string(abi.encodePacked("MockToken", Strings.toString(i))),
                string(abi.encodePacked("MT", Strings.toString(i))),
                18
            );
            _tokens[i] = address(mockToken);
            _weights[i] = 100;

            mockToken.mint(address(pod), 1000000 ether);
            mockToken.mint(address(this), 1000000 ether);
        }

        _immutables = abi.encode(
            address(dai),
            address(peas),
            address(dai),
            0x7d544DD34ABbE24C8832db27820Ff53C151e949b,
            rewardsWhitelist,
            0x024ff47D552cB222b265D68C7aeB26E586D5229D,
            dexAdapter
        );

        // Expect a revert with the "Too many tokens" message (or your custom message)
        vm.expectRevert("Too many tokens noni"); // Or your specific revert message

        podLarge.publicInit(_config, _tokens, _weights, _immutables);
    }
 

Add the provided TestWeightedIndex contract to your test folder. This contract exposes internal functions and logic from the __WeightedIndex_init function of the WeightedIndex contract, enabling thorough testing.

import "../contracts/WeightedIndex.sol";
import "../contracts/interfaces/IDecentralizedIndex.sol";

contract TestWeightedIndex is WeightedIndex {
    /// @notice Public wrapper to call __WeightedIndex_init for testing purposes.
    function publicInit(
        IDecentralizedIndex.Config memory _config,
        address[] memory _tokens,
        uint256[] memory _weights,
        bytes memory _immutables
    ) public {
        __WeightedIndex_init(_config, _tokens, _weights, _immutables);
    }

    /// @notice Returns the number of tokens stored in the indexTokens array.
    function indexTokenCount() public view returns (uint256) {
        return indexTokens.length;
    }

    /// @notice Returns the index stored in the _fundTokenIdx mapping for a given token.
    function getFundTokenIdx(address token) public view returns (uint256) {
        return _fundTokenIdx[token];
    }
}

Add the required mock contracts (e.g., MockERC20, MockUniswapV2Factory, etc.) to your mock folder. These mock contracts simulate the behavior of external dependencies and are essential for running the tests.

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

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "../contracts/interfaces/IPEAS.sol";


contract MockERC20 {
    string public name;
    string public symbol;
    uint8 public decimals;
    mapping(address => uint256) public balanceOf;
    mapping(address => mapping(address => uint256)) public allowance;

    constructor(string memory _name, string memory _symbol, uint8 _decimals) {
        name = _name;
        symbol = _symbol;
        decimals = _decimals;
    }

    function transfer(address recipient, uint256 amount) public returns (bool) {
        balanceOf[msg.sender] -= amount;
        balanceOf[recipient] += amount;
        return true;
    }

    function approve(address spender, uint256 amount) public returns (bool) {
        allowance[msg.sender][spender] = amount;
        return true;
    }

    // ... Add other mocked functions (like decimals, transferFrom, etc.) as required ...

    function mint(address to, uint256 amount) public {
        balanceOf[to] += amount;
    }

    // function decimals() public view returns (uint8) { // Add decimals function
    //     return decimals;
    // }
}


interface IUniswapV2Router02 {
    function WETH() external view returns (address);
    function factory() external view returns (address);
}

contract MockUniswapV2Router is IUniswapV2Router02 {
    address public WETH;
    address public factory;

    constructor(address _weth, address _factory) {
        WETH = _weth;
        factory = _factory;
    }

    // function WETH() external view returns (address) {
    //     return WETH;
    // }

    // function factory() external view returns (address) {
    //     return factory;
    // }

    // ... other functions as needed
}

contract MockPEAS is IPEAS, ERC20 {
    //uint8 public _decimals; // Store decimals as a state variable

    constructor(string memory _name, string memory _symbol, uint8 /* _decimalsValue */) 
        ERC20(_name, _symbol)
    {
        _mint(msg.sender, 10_000_000 * 10 ** 18); // Mint to the deployer for testing
        // Do not store any additional decimals value; rely on ERC20's default.
    }

    function burn(uint256 _amount) external virtual override {
        _burn(msg.sender, _amount); // Burn from the test contract (msg.sender)
        emit Burn(msg.sender, _amount);
    }

    // function decimals() public view virtual override returns (uint8) {
    //     return _decimals; // Return the stored decimals value
    // }

    // Add a mint function for testing purposes:
    function mint(address _to, uint256 _amount) public {
        _mint(_to, _amount);
    }

    // Add a setDecimals function to allow changing the decimals value for testing:
    // function setDecimals(uint8 _newDecimals) public {
    //     _decimals = _newDecimals;
    // }

    // ... other functions as needed for your tests ...
}

contract MockUniswapV2Factory {
    mapping(address => mapping(address => address)) public getPair;

    function setPair(address tokenA, address tokenB, address pairAddress) public {
        getPair[tokenA][tokenB] = pairAddress;
        getPair[tokenB][tokenA] = pairAddress;
    }

    // Simple createPair that deploys a new pair and stores it.
    function createPair(address tokenA, address tokenB) public returns (address pair) {
        MockUniswapV2Pair newPair = new MockUniswapV2Pair(tokenA, tokenB);
        pair = address(newPair);
        setPair(tokenA, tokenB, pair);
    }
}

// Mock Uniswap V2 Pair.
contract MockUniswapV2Pair {
    IERC20 public token0;
    IERC20 public token1;

    constructor(address _tokenA, address _tokenB) {
        token0 = IERC20(_tokenA);
        token1 = IERC20(_tokenB);
    }

    // ... other pair functionality as needed for your tests
}

Add the below getter function to access the WeightedIndex contract for _totalWeights private variable`

function totalWeights() public view returns (uint256) {
        return _totalWeights;
    }

The provided test output demonstrates the high gas usage associated with the __WeightedIndex_init function when called with the maximum number of tokens (255). This excessive gas consumption highlights the vulnerability and justifies the need for the implemented mitigations. The gas usage is prohibitively high for practical use and poses a significant risk of Denial-of-Service attacks.

Image

As shown above, the __WeightedIndex_init function has been observed to consume approximately 126.7 million gas when executed with the maximum number of tokens. This consumption far exceeds the typical Ethereum block gas limit (around 30–32 million gas). As a result, an attacker could exploit this behaviour by initiating the function with parameters that force maximum gas usage, effectively preventing the function from being executed within a block and causing a denial-of-service (DoS) condition for Pod initialization.

The included test output demonstrates the successful execution of the test_DOS_mitigation function. This output confirms that the mitigation prevents the vulnerable behavior and that the contract now correctly handles attempts to initialize with an excessive number of tokens. This output is only achievable after the recommended mitigation has been implemented in the contract. Image

Mitigation

  1. Limit the Number of Tokens: Implement a check within __WeightedIndex_init to limit the number of tokens that can be added in a single transaction:
require(_tokens.length <= MAX_TOKENS, "Too many tokens"); // Define MAX_TOKENS

Choose a reasonable MAX_TOKENS value (e.g., 10-20) that balances functionality and security.

  1. Pagination or Batching: Implement pagination or batching to allow adding a large number of tokens across multiple transactions.
  2. Gas Optimization: Optimize the gas-intensive operations within the loop, especially the q1 calculation. Consider caching values, using more gas-efficient libraries, or rewriting the logic if possible.
  3. Gas Limit per Transaction: Consider setting a maximum gas limit for calls to the __WeightedIndex_init function. This won't prevent a griefing attack but may limit the damage.
  4. State Growth Considerations: Implement strategies to manage state growth, such as removing or archiving old data if it's no longer needed. This is a longer-term consideration for any contract that stores data on-chain.