c4-bot-4 commented 6 months ago

Lines of code

Vulnerability details

Impact

Pendle Aura Balancer and other markets have the potential to disrupt all functions within PendlePowerFarmToken that depend on the syncSupply() modifier. This is because certain contracts within the protocol do not account for the possibility of new reward tokens being introduced in existing markets.

Proof of Concept

PendlePowerFarmController::addPendleMarket() currently initializes pendleChildCompoundInfo[_pendleMarket].lastIndex as a dynamic fixed-length array based on the length returned from IPendleMarket(_pendleMarket).getRewardTokens() at the time of the initialization.

function addPendleMarket(
    address _pendleMarket,
    string memory _tokenName,
    string memory _symbolName,
    uint16 _maxCardinality
)
    external
    onlyMaster
{
    if (pendleChildAddress[_pendleMarket] > ZERO_ADDRESS) {
        revert AlreadySet();
    }

    address pendleChild = PENDLE_POWER_FARM_TOKEN_FACTORY.deploy(
        _pendleMarket,
        _tokenName,
        _symbolName,
        _maxCardinality
    );

    pendleChildAddress[_pendleMarket] = pendleChild;

    _setRewardTokens(
        _pendleMarket,
        _getRewardTokens(
            _pendleMarket
        )
    );

    CompoundStruct storage childInfo = pendleChildCompoundInfo[
        _pendleMarket
    ];

    uint256 rewardTokensLength = childInfo
        .rewardTokens
        .length;

    childInfo.lastIndex = new uint128[](
        rewardTokensLength
    );

    childInfo.reservedForCompound = new uint256[](
        rewardTokensLength
    );

    uint256 i;

    while (i < rewardTokensLength) {

        address token = childInfo.rewardTokens[i];

        childInfo.lastIndex[i] = _getUserRewardIndex(
            _pendleMarket,
            token,
            address(this)
        );

        childInfo.reservedForCompound[i] = 0;

        _checkFeed(
            token
        );

        unchecked {
            ++i;
        }
    }

    _checkFeed(
        _pendleMarket
    );

    activePendleMarkets.push(
        _pendleMarket
    );

    emit AddPendleMarket(
        _pendleMarket,
        pendleChild
    );
}

This will work fine but has a risk of breaking because Pendle Aura Balancer and some other markets can have new reward tokens introduced at any time. Here's, for example, PendleAuraBalancerStableLPSYV3Upg's source:

function addRewardTokens(address token) external virtual onlyOwner {
    extraRewards.push(token);
}

function extraRewardsLength() external view virtual returns (uint256) {
    return extraRewards.length;
}

function _getRewardTokens() internal view virtual override returns (address[] memory res) {
    uint256 extraRewardsLen = extraRewards.length;
    res = new address[](2 + extraRewardsLen);
    res[0] = BAL_TOKEN;
    res[1] = AURA_TOKEN;
    for (uint256 i = 0; i < extraRewardsLen; i++) {
        res[2 + i] = extraRewards[i];
    }
}

All functions using the syncSupply modifier inside PendlePowerFarmToken will get DOSed when _overWriteCheck() is called:

modifier syncSupply()
{
    // ...
    _overWriteCheck();
    // ...
}

function _overWriteCheck()
    internal
{
    _wrapOverWrites(
        _updateRewardTokens()
    );
}

function _wrapOverWrites(
    bool _overWritten
)
    internal
{
    if (_overWritten == true) {
        _overWriteIndexAll();
        _overWriteAmounts();
    }
}

function _updateRewardTokens()
    private
    returns (bool)
{
    return PENDLE_CONTROLLER.updateRewardTokens(
        UNDERLYING_PENDLE_MARKET
    );
}

function _overWriteIndexAll()
    private
{
    PENDLE_CONTROLLER.overWriteIndexAll(
        UNDERLYING_PENDLE_MARKET
    );
}

function _overWriteIndex(
    uint256 _index
)
    private
{
    PENDLE_CONTROLLER.overWriteIndex(
        UNDERLYING_PENDLE_MARKET,
        _index
    );
}

function _overWriteAmounts()
    private
{
    PENDLE_CONTROLLER.overWriteAmounts(
        UNDERLYING_PENDLE_MARKET
    );
}

The pieces we're primarily interested in here are PENDLE_CONTROLLER.updateRewardTokens() and PENDLE_CONTROLLER.overWriteIndex():

function updateRewardTokens(
    address _pendleMarket
)
    external
    onlyChildContract(_pendleMarket)
    returns (bool)
{
    address[] memory rewardTokens = _getRewardTokens(
        _pendleMarket
    );

    if (_compareHashes(_pendleMarket, rewardTokens) == true) {
        return false;
    }

    _setRewardTokens(
        _pendleMarket,
        rewardTokens
    );

    emit UpdateRewardTokens(
        _pendleMarket,
        rewardTokens
    );

    return true;
}

function overWriteIndex(
    address _pendleMarket,
    uint256 _tokenIndex
)
    public
    onlyChildContract(_pendleMarket)
{
    CompoundStruct storage childInfo = pendleChildCompoundInfo[
        _pendleMarket
    ];

    childInfo.lastIndex[_tokenIndex] = _getUserRewardIndex(
        _pendleMarket,
        childInfo.rewardTokens[_tokenIndex],
        address(this)
    );
}

function overWriteIndexAll(
    address _pendleMarket
)
    external
    onlyChildContract(_pendleMarket)
{
    uint256 i;
    uint256 length = pendleChildCompoundInfo[
        _pendleMarket
    ].rewardTokens.length;

    while (i < length) {
        overWriteIndex(
            _pendleMarket,
            i
        );
        unchecked {
            ++i;
        }
    }
}

_setRewardTokens() comes from the helper contract PendlePowerFarmControllerHelper that the controller inherits:

function _setRewardTokens(address _pendleMarket, address[] memory _rewardTokens) internal {
    pendleChildCompoundInfo[_pendleMarket].rewardTokens = _rewardTokens;
}

As we can see overWriteIndexAll() is focused on aligning pendleChildCompoundInfo[_pendleMarket].lastIndex's values with the ones from the Pendle market. As seen in PendlePowerFarmController::addPendleMarket(), the last index array was initialized with a fixed length, so any new index overWriteIndexAll() tries to insert at will result in a revert unless added through a push().

Tools Used

Manual Review

Recommended Mitigation Steps

Refactor PendlePowerFarmController::overWriteIndex() so it uses push() for the newly added reward tokens.

Assessed type

DoS

GalloDaSballo commented 6 months ago

Logically adjacent to #135

c4-pre-sort commented 6 months ago

GalloDaSballo marked the issue as sufficient quality report

c4-pre-sort commented 6 months ago

GalloDaSballo marked the issue as primary issue

trust1995 commented 5 months ago

Confirmed OOS

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Out of scope

code-423n4 / 2024-02-wise-lending-findings

Pendle Aura Balancer market can permanently DOS `PendlePowerFarmToken` when new reward assets get added #124