0x52 - Adversary can brick AutoRoller by creating another AutoRoller on the same adapter

[sherlock-admin]

0x52

high

Adversary can brick AutoRoller by creating another AutoRoller on the same adapter

Summary

onSponsorWindowOpened attempts to make a new series at the desired maturity. Each adapter can only have one of each maturity. If the maturity requested already exists then onSponsorWindowOpened will revert, making it impossible to roll the AutoRoller. An adversary can take advantage of this to brick an AutoRoller by creating a second AutoRoller on the same adapter that will create a target maturity before the first AutoRoller. Since the maturity now exists, the first AutoRoller will always revert when trying to Roll.

Vulnerability Detail

uint256 _maturity = utils.getFutureMaturity(targetDuration);

function getFutureMaturity(uint256 monthsForward) public view returns (uint256) {
    (uint256 year, uint256 month, ) = DateTime.timestampToDate(DateTime.addMonths(block.timestamp, monthsForward));
    return DateTime.timestampFromDateTime(year, month, 1 /* top of the month */, 0, 0, 0);
}

Inside AutoRoller#onSponsorWindowOpened the maturity is calculated using RollerUtils#getFutureMaturity. This returns the timestamp the requested months ahead, truncated down to the first of the month. It passes this calculated maturity as the maturity to sponsor a new series.

(ERC20 _pt, YTLike _yt) = periphery.sponsorSeries(address(adapter), _maturity, true);

https://etherscan.io/address/0xFff11417a58781D3C72083CB45EF54d79Cd02437#code#F1#L90

function sponsorSeries(
    address adapter,
    uint256 maturity,
    bool withPool
) external returns (address pt, address yt) {
    (, address stake, uint256 stakeSize) = Adapter(adapter).getStakeAndTarget();

    // Transfer stakeSize from sponsor into this contract
    ERC20(stake).safeTransferFrom(msg.sender, address(this), stakeSize);

    // Approve divider to withdraw stake assets
    ERC20(stake).approve(address(divider), stakeSize);

    (pt, yt) = divider.initSeries(adapter, maturity, msg.sender);

    // Space pool is always created for verified adapters whilst is optional for unverified ones.
    // Automatically queueing series is only for verified adapters
    if (verified[adapter]) {
        poolManager.queueSeries(adapter, maturity, spaceFactory.create(adapter, maturity));
    } else {
        if (withPool) {
            spaceFactory.create(adapter, maturity);
        }
    }
    emit SeriesSponsored(adapter, maturity, msg.sender);
}

periphery#sponsorSeries is called with true indicating to create a space pool for the newly created series.

https://etherscan.io/address/0x5f6e8e9C888760856e22057CBc81dD9e0494aA34#code#F1#L75

function create(address adapter, uint256 maturity) external returns (address pool) {
    address pt = divider.pt(adapter, maturity);
    _require(pt != address(0), Errors.INVALID_SERIES);
    _require(pools[adapter][maturity] == address(0), Errors.POOL_ALREADY_EXISTS);

    pool = address(new Space(
        vault,
        adapter,
        maturity,
        pt,
        ts,
        g1,
        g2,
        oracleEnabled
    ));

    pools[adapter][maturity] = pool;
}

We run into an issue inside SpaceFactory#create because it only allows a single pool per adapter/maturity. If a pool already exist then it will revert.

An adversary can abuse this revert to brick an existing AutoRoller. Assume AutoRoller A has a duration of 3 months. Its current maturity is December 1st 2022, when rolled it will attempt to create a series at March 1st 2023. An adversary could abuse this and create AutoRoller B with a maturity of 4 months. When they roll for the first time it will create a series with maturity at March 1st 2023. When AutoRoller A attempts to roll it will revert since a series already exists at March 1st 2023.

This conflict can happen accidentally if there is a monthly AutoRoller and a quarterly AutoRoller. It also hinders the viability of using an AutoRoller for an adapter that is popular because the series will likely have been created by the time the autoroller tries to roll into it.

Impact

AutoRollers will frequently be bricked

Code Snippet

AutoRoller.sol#L174-L272

Tool used

Manual Review

Recommendation

Requiring that the AutoRoller has to create the series seems overly restrictive and leads to a large number of issues. Attempting to join an a series that is already initialized could also lead to pool manipulation rates. It seems like a large refactoring is needed for the rolling section of the AutoRoller

[jparklev]

This is a great observation and a valid high severity finding. We have a fix in mind that we'll share once implemented – appreciate the submission

[jparklev]

Fix: sense-finance/auto-roller#20

[aktech297]

The fix is done by considering the trusted caller.
If the caller is trusted, contract allows for auto roller creation.
This fix will be valid till isTrusted[msg.sender] == true after first one is created.
Even the trusted caller should not be allowed to create auto roller with maturity less than the existing auto roller.