code-423n4 / 2023-07-tapioca-findings

15 stars 10 forks source link

Tapioca DAO Treasury may loose up to 25% of potential earnings in a rapid weekly singularity market volume growth #409

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/TapiocaOptionBroker.sol?#L382-L387

Vulnerability details

Tapioca DAO Treasury may loose up to 25% of potential earnings in a rapid weekly singularity market volume growth

Lets assume a situation, where the Topioca protocol has a strong weekly growth of value deposited in singularity markets and these singularity positions get locked in the tOLP contract. The eligibleTapAomunt calculated for a user in exerciseOption based on his oTAP is calculated counterintuativly in real time on the TapiocaOptionBroker:

    uint256 eligibleTapAmount = muldiv( 
            tOLPLockPosition.amount,
            gaugeTotalForEpoch,
            tOLP.getTotalPoolDeposited(tOLPLockPosition.sglAssetID) //<== This is problematic, because real time!
        );

where tOLP.getTotalPoolDeposited(tOLPLockPosition.sglAssetID) is calculated in the TapiocaOptionLiquidityProvision

    function getTotalPoolDeposited( 
        uint256 _sglAssetId
    ) external view returns (uint256) {
        return
            activeSingularities[sglAssetIDToAddress[_sglAssetId]]
                .totalDeposited;
    }

Where activeSingularities[sglAssetIDToAddress[_sglAssetId]].totalDeposited gets incremented and decremented when a user locks or unlocks a singularity position for this market in real time:

  function lock(...){
    ...
       activeSingularities[_singularity].totalDeposited += _amount;
    ...
  }

  function unlock(...){
    ...
       activeSingularities[_singularity].totalDeposited -= lockPosition.amount;  
    ...
  }

This vulnerability is explained easiest with an example:

The user flow can be like this - a user mints 100 USD0 and:

Lets assume that:

Its because yesterday, another user minted 200 additional USD0. Now the eligibleTapAmount is 116 TAPs(100USD0 * 350TAPs)/300USD0 and line eligibleTapAmount -= oTAPCalls[_oTAPTokenID][cachedEpoch]; reverts because 116 -175 is smaller than 0 (underflow)

Impact

Lets further assume that half of the users do this strategy of only exercising 50% of the weekly oTap volume beginning of the newEpoch. This means 25% of the possible sold TAPs are not sold. Lets assume that 50% of these users switch to exercise 100% beginning of the week, bringing the potential loss to 12.5 to 25%.

The damage is for the Tapioca DAO Treasury that sells less TAPs than it could, in my sample calculation up to 25% less. The actual damage of course cannot be told exactly.

As well this will lead to a lot of frustration with users, because a promise is not kept - in my example the user was promised 350 TAPs but actually he was able to buy only 175. This can go to the user forums and from there to the media, slowing down user adoption.

Because it is quite certain that this scenario happens and a lot of value can be lost I rate this vulnerability as high.

Proof of Concept

I created a proof of concept named "should fail to claim partial exerciseOption in rising singularity markets". It behaves like this:

The script outputs the following lines.

The weekly emissions for singularity market 1 are: 312771.976

User0 deposited % into singularity market 1.  3.0

The eligible amount for user0 BEFORE deposit is: 312771.976
User0 already exercised: 0.0
payment tokens needed: 516073.7604

User0 exercised 50% his option for an Amount of TAPs: 156385.988

User 1 deposited: 2.0

The eligible amount for user0 AFTER first user 1 deposit is: 31277.1976
User0 already exercised: 156385.988
payment tokens needed: 51607.37604

User 1 deposited: 3.0

And here the script

  it('should fail to claim partial exerciseOption in rising singularity markets', async () => {
        const {
            users,
            yieldBox,
            tOB,
            tapOFT,
            tOLP,
            oTAP,
            sglTokenMock,
            sglTokenMockAsset,
            sglTokenMock2,
            sglTokenMock2Asset,
            stableMock,
            stableMockOracle,
        } = await loadFixture(setupFixture);

        await setupEnv(
            tOB,
            tOLP,
            tapOFT,
            sglTokenMock,
            sglTokenMockAsset,
            sglTokenMock2,
            sglTokenMock2Asset,
        );

        await tOB.oTAPBrokerClaim(); // only if it is not already called setupEnv.
        await tOLP.setSGLPoolWEight(sglTokenMock.address, 2);

        await tOB.setPaymentToken(
            stableMock.address,
            stableMockOracle.address,
            '0x00',
        );

        //get new epoch - Here the attack happens!
        tOB.newEpoch();

        // Gauge emission check
        const epoch = await tOB.epoch();
        const weeklyEmissions = await tapOFT.getCurrentWeekEmission();
        const sglGauge1Emissions = (weeklyEmissions)
            .mul(2)
            .div(3);
        const sglGauge2Emissions = (weeklyEmissions)
            .mul(1)
            .div(3);
        expect(await tOB.singularityGauges(epoch, sglTokenMockAsset)).to.equal(
            sglGauge1Emissions,
        );
        expect(await tOB.singularityGauges(epoch, sglTokenMock2Asset)).to.equal(
            sglGauge2Emissions,
        );

        console.log("The weekly emissions for singularity market 1 are: " + hre.ethers.utils.formatUnits(sglGauge1Emissions));
        console.log("");

        // check what happens if in this condition a user tried to exercise a oTAP
        // deposit to yieldbox
        await sglTokenMock.connect(users[0]).freeMint(BN(3e18));
        await sglTokenMock.connect(users[0]).approve(yieldBox.address, BN(3e18));
        await yieldBox
            .connect(users[0])
            .depositAsset(
                sglTokenMockAsset,
                users[0].address,
                users[0].address,
                BN(3e18),
                0,
            );

        const ybAmount = await yieldBox.toAmount(
            sglTokenMockAsset,
            await yieldBox.balanceOf(users[0].address, sglTokenMockAsset),
            false,
        );

        console.log("User0 deposited % into singularity market 1. " , hre.ethers.utils.formatUnits(ybAmount.toString()));
        console.log("");

        // lock to TapiocaOptionLiquidityProvider
        await yieldBox.connect(users[0]).setApprovalForAll(tOLP.address, true);
        await tOLP.connect(users[0])
            .lock(users[0].address, sglTokenMock.address, 1_209_600, ybAmount);//2 weeks
        const tOLPTokenID = await tOLP.tokenCounter();

        // create a oTAP
        await tOLP.connect(users[0]).approve(tOB.address, tOLPTokenID);
        await tOB.connect(users[0]).participate(tOLPTokenID);
        const oTAPTokenID = await oTAP.mintedOTAP();
        const oTAPOption = await oTAP.options(oTAPTokenID);

        // User 0 checks what TAPs he is eligible 

        const otcDetails = await tOB.connect(users[0]).getOTCDealDetails(oTAPTokenID,stableMock.address,0);

        const eligibleTapAmount = otcDetails.eligibleTapAmount;
        const paymentTokenToSend =otcDetails.paymentTokenAmount;

        console.log("The eligible amount for user0 BEFORE deposit is: " + hre.ethers.utils.formatUnits(eligibleTapAmount));
        console.log("User0 already exercised: " + hre.ethers.utils.formatUnits(await tOB.connect(users[0]).oTAPCalls(oTAPTokenID,epoch)));
        console.log("payment tokens needed: " + hre.ethers.utils.formatUnits(paymentTokenToSend));
        console.log("");

        // Exercise option for for half of his eligible amount
        await stableMock.mintTo(users[0].address, BN(paymentTokenToSend));
        await stableMock.connect(users[0]).approve(tOB.address, BN(paymentTokenToSend));

        await expect(tOB.connect(users[0]).exerciseOption(
                    oTAPTokenID,
                    stableMock.address,
                    BN(eligibleTapAmount).div(2),
                )).to.emit(tOB, 'ExerciseOption')
                .withArgs(
            epoch,
            users[0].address,
            stableMock.address,
            oTAPTokenID,
            BN(eligibleTapAmount).div(2),
        ); // Successful exercise

        console.log("User0 exercised 50% his option for an Amount of TAPs: " + hre.ethers.utils.formatUnits(await tapOFT.balanceOf(users[0].address)));
        console.log("");

        // user 1 arrives and deposit to yieldbox
        await sglTokenMock.connect(users[1]).freeMint(BN(6e18));
        await sglTokenMock.connect(users[1]).approve(yieldBox.address, BN(6e18));
        await yieldBox
            .connect(users[1])
            .depositAsset(
                sglTokenMockAsset,
                users[1].address,
                users[1].address,
                BN(2e18),
                0,
            );

        const ybAmount1 = await yieldBox.toAmount(
            sglTokenMockAsset,
            await yieldBox.balanceOf(users[1].address, sglTokenMockAsset),
            false,
        );

        // lock to TapiocaOptionLiquidityProvider
        await yieldBox.connect(users[1]).setApprovalForAll(tOLP.address, true);
        await tOLP.connect(users[1])
            .lock(users[1].address, sglTokenMock.address, 1_209_600, ybAmount1);//2 weeks
        const tOLPTokenID1 = await tOLP.tokenCounter();

        // create a oTAP
        await tOLP.connect(users[1]).approve(tOB.address, tOLPTokenID1);
        await tOB.connect(users[1]).participate(tOLPTokenID1);
        const oTAPTokenID1 = await oTAP.mintedOTAP();
        const oTAPOption1 = await oTAP.options(oTAPTokenID1);

        console.log("User 1 deposited: " + hre.ethers.utils.formatUnits(ybAmount1.toString()));
        console.log("");

        //now user 2 wants to exercise the second part of his eligible option, but it fails
        const otcDetails1 =  await tOB.connect(users[0]).getOTCDealDetails(oTAPTokenID,stableMock.address,0);

        const eligibleTapAmount1 = otcDetails1.eligibleTapAmount;
        const paymentTokenToSend1 = otcDetails1.paymentTokenAmount;

        console.log("The eligible amount for user0 AFTER first user 1 deposit is: " + hre.ethers.utils.formatUnits(eligibleTapAmount1));
        console.log("User0 already exercised: " + hre.ethers.utils.formatUnits(await tOB.connect(users[0]).oTAPCalls(oTAPTokenID,epoch)));
        console.log("payment tokens needed: " + hre.ethers.utils.formatUnits(paymentTokenToSend1));
        console.log("");

        await expect(tOB.connect(users[0]).exerciseOption(
            oTAPTokenID,
            stableMock.address,
            BN(eligibleTapAmount).div(2),//one tapOFT
        )).to.emit(tOB, 'ExerciseOption')
        .withArgs(
            epoch,
            users[0].address,
            stableMock.address,
            oTAPTokenID,
            BN(eligibleTapAmount).div(2),
        ).to.be.revertedWith("tOB: Too high")

        //now the user 1 deposits another amount of 3e8
        await yieldBox
            .connect(users[1])
            .depositAsset(
                sglTokenMockAsset,
                users[1].address,
                users[1].address,
                BN(3e18),
                0,
            );

        const ybAmount2 = await yieldBox.toAmount(
            sglTokenMockAsset,
            await yieldBox.balanceOf(users[1].address, sglTokenMockAsset),
            false,
        );

        console.log("User 1 deposited: " + hre.ethers.utils.formatUnits(ybAmount2.toString()));
        console.log("");

        // lock to TapiocaOptionLiquidityProvider
        await yieldBox.connect(users[1]).setApprovalForAll(tOLP.address, true);
        await tOLP.connect(users[1])
            .lock(users[1].address, sglTokenMock.address, 1_209_600, ybAmount2);//2 weeks
        const tOLPTokenID2 = await tOLP.tokenCounter();

        // create a oTAP
        await tOLP.connect(users[1]).approve(tOB.address, tOLPTokenID2);
        await tOB.connect(users[1]).participate(tOLPTokenID2);
        const oTAPTokenID2 = await oTAP.mintedOTAP();
        const oTAPOption2 = await oTAP.options(oTAPTokenID2);

        // now user 0 wants to know how much taps he is eligile of, but it fails
        // because the eligible amount is smaller than the already claimed amount
        // to run this, please comment out, some hardhat versions do not work with reverdedWithPanic
        const otcDetails2 =  expect(await tOB.connect(users[0])
            .getOTCDealDetails(oTAPTokenID,stableMock.address,0)).to.be
            .revertedWithPanic(PANIC_CODES.ARITHMETIC_UNDER_OR_OVERFLOW);

        // the same thing happens when the user tries to exercise his option:
        await expect(tOB.connect(users[0]).exerciseOption(
            oTAPTokenID,
            stableMock.address,
            BN(eligibleTapAmount).div(2),//one tapOFT
        )).to.emit(tOB, 'ExerciseOption')
        .withArgs(
            epoch,
            users[0].address,
            stableMock.address,
            oTAPTokenID,
            BN(eligibleTapAmount).div(2),
        ).to.be.revertedWithPanic(PANIC_CODES.ARITHMETIC_UNDER_OR_OVERFLOW);
        //revertedWithPanic does not work on my machine, I receive the exception
        //Error: call revert exception; VM Exception while processing transaction: reverted with panic code 17
    });

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Using the current total singularity market volume to calculate the eligibleTapAmount is very dangerous. In another vulnerability I show that this enables another attack (and there might be more). I highly recommend to snapshot the volumes of all singularity markets every week pool volume when newEpoch is called. Then use these snapshotted singularity market volumes to calculate the eligibleTapAmount.

Assessed type

Under/Overflow

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-sponsor commented 1 year ago

0xRektora marked the issue as sponsor disputed

0xRektora commented 1 year ago

That's part of the design. First come first serve, it incentivize users to; A) take actions quickly B) Small amounts will get crushed by larger amounts, this is to de-incentivize spam by removing any benefit to it.

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid