Vulnerability to DoS Attacks via Dust Transactions in Position Management Functions

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xc454e59a3a6bcded06ae61257c38a5cf98255737ede752b8a7c454efb38dab97 Severity: medium

Description: Description

The Router, MainnetRouter, and GnosisRouter contracts contain functions for managing positions (splitting, merging, and redeeming) that do not implement minimum amount checks or other safeguards against small transactions. This allows users to perform transactions with arbitrarily small amounts, including dust amounts.

A malicious actor could exploit this vulnerability by repeatedly calling these functions with minimal amounts:

1. In Router.sol, an attacker could repeatedly call splitPosition() or mergePositions() with very small amount values. For example:

   router.splitPosition(token, market, 1); // Using smallest possible unit

2. In MainnetRouter.sol, an attacker could flood the network with tiny DAI transactions:

   mainnetRouter.splitFromDai(market, 1); // Using 1 wei of DAI
   mainnetRouter.mergeToDai(market, 1);

3. In GnosisRouter.sol, an attacker could send numerous transactions with minimal xDAI:

   gnosisRouter.splitFromBase{value: 1}(market); // Sending 1 wei of xDAI
   gnosisRouter.mergeToBase(market, 1);

By executing these functions repeatedly with minimal amounts, an attacker could potentially congest the network, causing increased gas costs, transaction delays, and possibly preventing legitimate users from interacting with the protocol.

Proof of Concept (PoC) File

All showcased logic below lacks a minimum amount check, allowing dust transactions. As stated above an attacker could call this functions repeatedly with dust amounts to congest the network.

Router.sol:

function splitPosition(IERC20 collateralToken, Market market, uint256 amount) public {
    if (market.parentCollectionId() == bytes32(0)) {
        collateralToken.transferFrom(msg.sender, address(this), amount);
    }
    _splitPosition(collateralToken, market, amount);
}

function mergePositions(IERC20 collateralToken, Market market, uint256 amount) public {
    _mergePositions(collateralToken, market, amount);
    if (market.parentCollectionId() == bytes32(0)) {
        collateralToken.transfer(msg.sender, amount);
    }
}

MainnetRouter.sol:

function splitFromDai(Market market, uint256 amount) external {
    DAI.transferFrom(msg.sender, address(this), amount);
    DAI.approve(address(sDAI), amount);
    uint256 shares = sDAI.deposit(amount, address(this));
    _splitPosition(IERC20(address(sDAI)), market, shares);
}

function mergeToDai(Market market, uint256 amount) external {
    _mergePositions(IERC20(address(sDAI)), market, amount);
    sDAI.redeem(amount, msg.sender, address(this));
}

GnosisRouter.sol:

function splitFromBase(Market market) external payable {
    uint256 shares = savingsXDaiAdapter.depositXDAI{value: msg.value}(address(this));
    _splitPosition(sDAI, market, shares);
}

function mergeToBase(Market market, uint256 amount) external {
    _mergePositions(sDAI, market, amount);
    sDAI.approve(address(savingsXDaiAdapter), amount);
    savingsXDaiAdapter.redeemXDAI(amount, msg.sender);
}

Recommended Mitigations

To protect against potential DoS attacks through dust transactions, the protocol can implement some of the following measures:

1. Introduce a minimum transaction amount:

   uint256 public minTransactionAmount;
   
   function splitPosition(IERC20 collateralToken, Market market, uint256 amount) public {
      require(amount >= minTransactionAmount, "Amount too low");
      // Rest of the function...
   }

2. Make the minimum amount configurable:

   function setMinTransactionAmount(uint256 _newAmount) external onlyOwner {
      require(_newAmount > 0, "Invalid minimum amount");
      minTransactionAmount = _newAmount;
      emit MinTransactionAmountUpdated(_newAmount);
   }

3. Use a gas price floor:

   uint256 public minGasPrice;
   
   function splitPosition(IERC20 collateralToken, Market market, uint256 amount) public {
      require(tx.gasprice >= minGasPrice, "Gas price too low");
      // Rest of the function...
   }
   
   function setMinGasPrice(uint256 _newMinGasPrice) external onlyOwner {
      minGasPrice = _newMinGasPrice;
      emit MinGasPriceUpdated(_newMinGasPrice);
   }

By implementing one or more of these measures, the protocol can significantly reduce its vulnerability to dust transaction-based DoS attacks while maintaining functionality for legitimate users.

[greenlucid]

Those interactions do not increase the cost, other than raising gas costs for everyone in the chosen network. If there's a way those dust attacks can increase the gas cost of legitimate users (without involving DoSing the entire blockchain), it's on you to prove how exactly it raises gas costs. Because actually I think that if you dust someone they'll even be able to do those transactions CHEAPER, not more expensive, thanks to non-zero -> non-zero SSTOREs.

[clesaege]

Yeah, gas usage is at platform level and it's well known you can spam TX to increase gas cost (for everyone, but by spending a crazy amount of money yourself).

Per contest rules, are excluded:

• Issues about the underlying platform (in particular we assume that user transactions always get included within a reasonable amount of time).