coffiasd - User can get much less ZVE than expected

[sherlock-admin2]

coffiasd

high

User can get much less ZVE than expected

Summary

User deposit stable coin say DAI to ZivoeTranches.sol can get ZVE as reward , however the amount of ZVE can be mu ch less than expected

Vulnerability Detail

According to the documentation at https://docs.zivoe.com/how-to-guides/minting-tranche-tokens, users can preview the amount of ZVE before depositing stablecoins into the contract via the functions rewardZVEJuniorDeposit or rewardZVESeniorDeposit. However, the actual mint rate of ZVE can be changed based on the value of avgRate. If avgRatio exceeds upperRatioIncentiveBIPS, the value of avgRate resets to minZVEPerJTTMint. Assuming minZVEPerJTTMint is ZERO, users receive ZERO ZVE rewards. This may exceed the expected usage.

add test to file Test_ZivoeTranches.sol:

function test_deposit_with_preview() public {
    simulateITO(100 ether, 100 ether, 100 * USD, 100 * USD);

    mint("DAI", address(sam), 1000 ether);
    assert(sam.try_approveToken(address(DAI), address(ZVT), 1000 ether));

    god.try_updateMaxZVEPerJTTMint(address(ZVT), 0.3 * 10**18);
    god.try_updateUpperRatioIncentiveBIPS(address(ZVT),2100);

    hevm.startPrank(address(sam));
    ZVT.depositSenior(100 ether, address(DAI));

    //sam can preview the amount of zve before deposit.
    uint256 samZVEBefore = IERC20(address(ZVE)).balanceOf(address(sam));
    console2.log("sam zve before:",samZVEBefore);
    console2.log("sam preview zve:",ZVT.rewardZVEJuniorDeposit(10 ether));
    hevm.stopPrank();

    //bob front run to deposit DAI 
    hevm.startPrank(address(bob));
    mint("DAI", address(bob), 60 ether);
    assert(bob.try_approveToken(address(DAI), address(ZVT), 60 ether));
    ZVT.depositJunior(60 ether, address(DAI));
    console2.log("bob get zve:",IERC20(address(ZVE)).balanceOf(address(bob)));
    hevm.stopPrank();

    //sam deposit DAI.
    hevm.startPrank(address(sam));
    ZVT.depositJunior(20 ether, address(DAI));
    uint256 samZVEAfter = IERC20(address(ZVE)).balanceOf(address(sam));
    console2.log("sam actual get zve:",samZVEAfter - samZVEBefore);
}

output:

[PASS] test_deposit_with_preview() (gas: 2780284)
Logs:
  sam zve before: 25663636363636363600
  sam preview zve: 515454545454545460
  bob get zve: 687272727272727300
  sam actual get zve: 0

We can see that before Sam deposits DAI, he can preview the amount of ZVE. However, Bob front-runs his transaction and deposits 60 DAI to get the same ZVE reward. Due to the avgRate, Sam gets ZERO ZVE as a reward.

Impact

User can get much less ZVE than expected

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/ZivoeTranches.sol#L203-L229

function rewardZVEJuniorDeposit(uint256 deposit) public view returns (uint256 reward) {

    (uint256 seniorSupp, uint256 juniorSupp) = IZivoeGlobals_ZivoeTranches(GBL).adjustedSupplies();

    uint256 avgRate;    // The avg ZVE per stablecoin deposit reward, used for reward calculation.

    uint256 diffRate = maxZVEPerJTTMint - minZVEPerJTTMint;

    uint256 startRatio = juniorSupp * BIPS / seniorSupp;
    uint256 finalRatio = (juniorSupp + deposit) * BIPS / seniorSupp;
    uint256 avgRatio = (startRatio + finalRatio) / 2;

    if (avgRatio <= lowerRatioIncentiveBIPS) {
        avgRate = maxZVEPerJTTMint;
    } else if (avgRatio >= upperRatioIncentiveBIPS) {
        avgRate = minZVEPerJTTMint;
    } else {
        avgRate = maxZVEPerJTTMint - diffRate * (avgRatio - lowerRatioIncentiveBIPS) / (upperRatioIncentiveBIPS - lowerRatioIncentiveBIPS);
    }

    reward = avgRate * deposit / 1 ether;

    // Reduce if ZVE balance < reward.
    if (IERC20(IZivoeGlobals_ZivoeTranches(GBL).ZVE()).balanceOf(address(this)) < reward) {
        reward = IERC20(IZivoeGlobals_ZivoeTranches(GBL).ZVE()).balanceOf(address(this));
    }
}

Tool used

Foundry

Recommendation

We should add slippage protection for the amount of ZVE users receive.

Duplicate of #63

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

panprog commented:

borderline low/medium. dup of #63. The incentive is expected to be something extra, not the main driver of the deposit decision for the user, so it doesn't make much sense to add slippage control to incentive. Additionally, while Sherlock judging rules include slippage issues as valid, but only when this causes loss of funds (like uniswap pool manipulation to steal from the user who swaps without slippage control), small "price" (or incentive in this case) changes are not valid slippage issues, this is rather a "good feature to have" (informational issue), but not a security issue. Still, if the incentive is large enough, this can be important to user, hence borderline.