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

15 stars 10 forks source link

Tapioca Option Broker - No input validation for Epoch duration #503

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#L110

Vulnerability details

Impact

The Tapioca Option broker contracts plays a vital role since it enables users to lock their TOLP positions and mint an OTap Position. according to the specified system design the otap position then would enable the user to exercise the option to buy TAP tokens with a discount on a 'weekly' basis.

Indeed, the TAP token itself is designed to produce emissions on a weekly basis, as can be seen below:

    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;
}

However, the Option broker does not ensure that the epoch duration is 7 days, so it could be either by mistake of maliciously by owner set to a very arbitrarily large number. once it is set, it is also not modifiable by the owner.

// @audit notice comment suggests also it should be set to 7 days, but this is not enforced
uint256 public immutable EPOCH_DURATION; // 7 days = 604800\

If it is set to a very large number, lets say 10 weeks the impact is that the users will only be able to exercise their options and receive their TAP tokens every 10 weeks. this will be quite detrimental to users and is a clear delineation from the intended design of the Tapioca DSO incentive program described in their documentation.

    function newEpoch() external {
    // @audit because epoch duration in theory could be set to a very
    // large number that is more than a week, that means that
    // weekly emisions to gauges will not be
    // possible
    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 if epoch is set top high, then 
    // emitForWeek() needs to be manually triggered
    // outside the scope of this function in order to
    // ensure weekly emmisions are properly accumulated
    // otherwise users will lose out emissions
    uint256 epochTAP = tapOFT.emitForWeek();
    _emitToGauges(epochTAP);

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

the function above is responsible for updating the epoch in the option broker, it retrieves the current epoch emmisions(including accumulated unclaimed emissions). if the epoch duration is too high, this function will revert. this causes a few things.

  1. triggering a new epoch will not be possible and hence emissions to guages will not be done.
  2. emitForWeek in tap token will not be triggered weekly. this function must be triggered every week to keep track to accumulated unclaimed emissions. this can still be triggered by any third party, but must be done every week to ensure unclaimed tap tokens are accounted for, and then distributed to guages once the epoch(1 week or 10 weeks for that matter) starts.

while the impact is relatively high, it is minimised by the fact that the owner controls this variable in setup time, so they need to ensure the variable is actually set to 1 week on deployment.

Proof of Concept

Issue is self explanatory

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure sufficient input validation when initializing EPOCH_DURATION VARIABLE, or just set it as a constant with value of 1 week. uint256 public constant EPOCH_DURATION = 604800;

Assessed type

Invalid Validation

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

Consider QA

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

dmvt marked the issue as grade-b