code423n4 commented 12 months ago

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/TapiocaOptionBroker.sol#L426 https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/tokens/TapOFT.sol#L201

Vulnerability details

Impact

An attacker can prevent rewards from being issued to gauges for a given epoch

Proof of Concept

TapOFT.emitForWeek() is callable by anyone. The function will only return a value > 0 the first time it's called in any given week:

    ///-- Write methods --
    /// @notice Emit the TAP for the current week
    /// @return the emitted amount
    function emitForWeek() external notPaused returns (uint256) {
        require(_getChainId() == governanceChainIdentifier, "chain not valid");

        uint256 week = _timestampToWeek(block.timestamp);
        if (emissionForWeek[week] > 0) return 0;

        // Update DSO supply from last minted emissions
        dso_supply -= mintedInWeek[week - 1];

        // Compute unclaimed emission from last week and add it to the current week emission
        uint256 unclaimed = emissionForWeek[week - 1] - mintedInWeek[week - 1];
        uint256 emission = uint256(_computeEmission());
        emission += unclaimed;
        emissionForWeek[week] = emission;

        emit Emitted(week, emission);

        return emission;
    }

In TapiocaOptionBroker.newEpoch() the return value of emitForWeek() is used to determine the amount of tokens to distribute to the gauges. If the return value is 0, it will assign 0 reward tokens to each gauge:

    /// @notice Start a new epoch, extract TAP from the TapOFT contract,
    ///         emit it to the active singularities and get the price of TAP for the epoch.
    function newEpoch() external {
        require(
            block.timestamp >= lastEpochUpdate + EPOCH_DURATION,
            "tOB: too soon"
        );
        uint256[] memory singularities = tOLP.getSingularities();
        require(singularities.length > 0, "tOB: No active singularities");

        // Update epoch info
        lastEpochUpdate = block.timestamp;
        epoch++;

        // Extract TAP

        // @audit `emitForWeek` can be called by anyone. If it's called for a given
        // week, subsequent calls will return `0`. 
        // 
        // Attacker calls `emitForWeek` before it's executed through `newEpoch()`.
        // The call to `newEpoch()` will cause `emitForWeek` to return `0`.
        // That will prevent it from emitting any of the TAP to the gauges.
        // For that epoch, no rewards will be distributed to users.
        uint256 epochTAP = tapOFT.emitForWeek();
        _emitToGauges(epochTAP);

        // Get epoch TAP valuation
        (, epochTAPValuation) = tapOracle.get(tapOracleData);
        emit NewEpoch(epoch, epochTAP, epochTAPValuation);
    }

An attacker who frontruns the call to newEpoch() with a call to emitForWeek() will prevent any rewards from being distributed for a given epoch.

The reward tokens aren't lost. TapOFT will roll the missed epoch's rewards into the next one. Meaning, the gauge rewards will be delayed. The length depends on the number of times the attacker is able to frontrun the call to newEpoch().

But, it will cause the distribution to be screwed. If Alice is eligible for gauge rewards until epoch x + 1 (her lock runs out), and the attacker manages to keep the attack running until x + 2, she won't be able to claim her reward tokens. They will be distributed in epoch x + 3 to all the users who have an active lock at that time.

Here's a PoC:

// tOB.test.ts
    it.only("should fail to emit rewards to gauges if attacker frontruns", async () => {
        const {
            tOB,
            tapOFT,
            tOLP,
            sglTokenMock,
            sglTokenMockAsset,
            tapOracleMock,
            sglTokenMock2,
            sglTokenMock2Asset,
        } = await loadFixture(setupFixture);

        // Setup tOB
        await tOB.oTAPBrokerClaim();
        await tapOFT.setMinter(tOB.address);

        // No singularities
        await expect(tOB.newEpoch()).to.be.revertedWith(
            'tOB: No active singularities',
        );

        // Register sgl
        const tapPrice = BN(1e18).mul(2);
        await tapOracleMock.set(tapPrice);
        await tOLP.registerSingularity(
            sglTokenMock.address,
            sglTokenMockAsset,
            0,
        );

        await tapOFT.emitForWeek();

        await tOB.newEpoch();

        const emittedTAP = await tapOFT.getCurrentWeekEmission();

        expect(await tOB.singularityGauges(1, sglTokenMockAsset)).to.be.equal(
            emittedTAP,
        );
    })

Test output:

  TapiocaOptionBroker
    1) should fail to emit rewards to gauges if attacker frontruns

  0 passing (1s)
  1 failing

  1) TapiocaOptionBroker
       should fail to emit rewards to gauges if attacker frontruns:

      AssertionError: expected 0 to equal 469157964000000000000000. The numerical values of the given "ethers.BigNumber" and "ethers.BigNumber" inputs were compared, and they differed.
      + expected - actual

      -0
      +469157964000000000000000

      at Context.<anonymous> (test/oTAP/tOB.test.ts:606:73)
      at processTicksAndRejections (node:internal/process/task_queues:96:5)
      at runNextTicks (node:internal/process/task_queues:65:3)
      at listOnTimeout (node:internal/timers:528:9)
      at processTimers (node:internal/timers:502:7)

Tools Used

none

Recommended Mitigation Steps

emitForWeek() should return the current week's emitted amount if it was already called:

    function emitForWeek() external notPaused returns (uint256) {
        require(_getChainId() == governanceChainIdentifier, "chain not valid");

        uint256 week = _timestampToWeek(block.timestamp);
        if (emissionForWeek[week] > 0) return emissionForWeek[week];
        // ...

Assessed type

DoS

c4-pre-sort commented 11 months ago

minhquanym marked the issue as duplicate of #192

c4-judge commented 10 months ago

dmvt changed the severity to 3 (High Risk)