pool rescue timelock can be bypassed

[c4-bot-7]

Lines of code

https://github.com/Tapioca-DAO/tap-token/blob/20a83b1d2d5577653610a6c3879dff9df4968345/contracts/options/TapiocaOptionLiquidityProvision.sol#L275-L277

Vulnerability details

Description

In TapiocaOptionLiquidityProvision (tOLP) the owner can chose to set an asset in cooldown. This essentially closes down the asset and what has been locked against it, allowing users to withdraw ahead of their lock expiry.

This works under a cooldown where the owner must first call TapiocaOptionLiquidityProvision::requestSglPoolRescue:

File: tap-token/contracts/options/TapiocaOptionLiquidityProvision.sol

287:        sglRescueRequest[_sglAssetID] = block.timestamp;

This starts a timelock timer until activateSGLPoolRescue can be called which triggers the actual rescue mode:

File: tap-token/contracts/options/TapiocaOptionLiquidityProvision.sol

300:        if (block.timestamp < sglRescueRequest[sgl.sglAssetID] + rescueCooldown) revert RescueCooldownNotReached();
301:
302:        activeSingularities[singularity].rescue = true;

rescueCooldown is by default 2 days, but can be changed by the owner in setRescueCooldown:

File: tap-token/contracts/options/TapiocaOptionLiquidityProvision.sol

275:    function setRescueCooldown(uint256 _rescueCooldown) external onlyOwner {
276:        rescueCooldown = _rescueCooldown;
277:    }

The issue is that there is no cooldown on changing rescueCooldown. Hence the owner can call requestSglPoolRescue then set setRescueCooldown to 0 and lastly call activateSGLPoolRescue to instantly set the sglAsset in rescue mode.

Impact

The owner can bypass the rescue timelock by changing the cooldown, effectively triggering rescue in the same tx as requesting it. This breaks the invariant that an asset must wait a cooldown period before going into rescue mode. Thus removing the possibility for users to act on the changes.

Proof of Concept

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

pragma solidity 0.8.22;

import "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {IPearlmit} from "tapioca-periph/pearlmit/PearlmitHandler.sol";
import {TapiocaOptionLiquidityProvision} from "tap-token/options/TapiocaOptionLiquidityProvision.sol";

contract tOLPTest is Test {

    uint256 constant AssetId = 1;

    TapiocaOptionLiquidityProvision tOLP;

    IERC20 singularity = IERC20(makeAddr("Singularity"));

    function setUp() public {
        tOLP = new TapiocaOptionLiquidityProvision({
            _yieldBox: makeAddr("YieldBox"),
            _epochDuration: 1 weeks,
            _pearlmit: IPearlmit(makeAddr("Pearlmit")),
            _owner: address(this)
        });
        tOLP.registerSingularity(singularity, AssetId, 1);
    }

    function testActivateRescueImmediately() public {
        tOLP.requestSglPoolRescue(AssetId);

        // cant activate, since timelock has not passed
        vm.expectRevert(TapiocaOptionLiquidityProvision.RescueCooldownNotReached.selector);
        tOLP.activateSGLPoolRescue(singularity);

        // however owner can simply set timelock to 0, without any timelock
        tOLP.setRescueCooldown(0);

        // then activation works without any cooldown
        tOLP.activateSGLPoolRescue(singularity);
    }
}

Tools Used

Manual audit

Recommended Mitigation Steps

Consider implementing the same timelock for changing the rescue cooldown as for activating pool rescue.

Assessed type

Timing

[c4-sponsor]

cryptotechmaker marked the issue as disagree with severity

[cryptotechmaker]

Low

[c4-judge]

dmvt marked the issue as primary issue

[c4-judge]

dmvt marked the issue as duplicate of #127

[c4-judge]

dmvt marked the issue as selected for report

[c4-judge]

dmvt marked the issue as partial-75

[c4-judge]

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