code-423n4 / 2024-02-tapioca-findings

1 stars 1 forks source link

TAP tokens will be lost in the event that no singularities are registered for a week #143

Open c4-bot-8 opened 4 months ago

c4-bot-8 commented 4 months ago

Lines of code

https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/tokens/TapToken.sol#L397 https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/options/TapiocaOptionBroker.sol#L411-L412

Vulnerability details

Description

In the TapToken contract, the emitForWeek() function serves to emit the amount of TAP tokens to be minted in the current week, while also accumulating the unminted TAP tokens from the previous week.

function emitForWeek() external onlyMinter returns (uint256) {
    ...

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

    // Compute unclaimed emission from last week and add it to the current week emission
    uint256 unclaimed;
    if (week > 0) {
        dso_supply -= mintedInWeek[week - 1];
        uint256 unclaimed = emissionForWeek[week - 1] - mintedInWeek[week - 1];
    }
    uint256 emission = _computeEmission(); // [#explain] dso_supply * decay_rate / 1e18
    emission += unclaimed;

    ...
}

However, the calculation of week is based on the current timestamp. Therefore, if the function hasn't been called for one week or more, the unclaimed TAP tokens will not be accumulated for the next week.

For instance, if the contract isn't called throughout week 2, and then in week 3, the contract is invoked, the emitForWeek() function will not accumulate the unminted amount from week 1.

It's noteworthy that the emitForWeek() function can only be called by the minter, which is the TapiocaOptionBreaker contract, through the TapiocaOptionBreaker.newEpoch() function. However, this function is not restricted to who can call it, hence there might be scenarios where it is not triggered for a week. Additionally, if there are no active singularities registered in the tOLP contract, the newEpoch() function will revert with a NoActiveSingularities error. If this occurs during a week, the aforementioned issue will arise even there are some applications are used to trigger the function each week.

function newEpoch() external {
    if (_timestampToWeek(block.timestamp) <= epoch) revert TooSoon();

    uint256[] memory singularities = tOLP.getSingularities();
    if (singularities.length == 0) revert NoActiveSingularities();

    ...
}

Impact

There is a risk of losing some unclaimed TAP tokens from the previous week.

Tools Used

Manual review

Recommended Mitigation Steps

Instead of retrieving the unclaimed amount from week - 1, it should be obtained from the most recent week when emitForWeek() is triggered.

Assessed type

Other

c4-judge commented 3 months ago

dmvt marked the issue as primary issue

c4-sponsor commented 3 months ago

0xRektora (sponsor) acknowledged

c4-sponsor commented 3 months ago

0xRektora marked the issue as disagree with severity

0xRektora commented 3 months ago

The probability of this happening are close to nothing.

this function is not restricted to who can call it, hence there might be scenarios where it is not triggered for a week.

Anyone having the possibility to call is the opposite of this. One would be incentivized to participate early, on top of having the protocol automatically calling it.

c4-sponsor commented 3 months ago

0xRektora marked the issue as agree with severity

c4-sponsor commented 3 months ago

0xRektora (sponsor) disputed

0xRektora commented 3 months ago

Dupe https://github.com/code-423n4/2024-02-tapioca-findings/issues/38

c4-judge commented 3 months ago

dmvt marked the issue as duplicate of #38

c4-judge commented 3 months ago

dmvt changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

dmvt marked the issue as grade-a