sherlock-audit / 2023-03-sense-judging

4 stars 0 forks source link

0x52 - Periphery#_swapPTsForTarget won't work correctly if PT is mature but redeem is restricted #32

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

0x52

medium

Periphery#_swapPTsForTarget won't work correctly if PT is mature but redeem is restricted

Summary

Periphery#_swapPTsForTarget doesn't properly account for mature PTs that have their redemption restricted

Vulnerability Detail

Periphery.sol#L531-L551

function _swapPTsForTarget(
    address adapter,
    uint256 maturity,
    uint256 ptBal,
    PermitData calldata permit
) internal returns (uint256 tBal) {
    _transferFrom(permit, divider.pt(adapter, maturity), ptBal);

    if (divider.mscale(adapter, maturity) > 0) {
        tBal = divider.redeem(adapter, maturity, ptBal); <- @audit-issue always tries to redeem even if restricted
    } else {
        tBal = _balancerSwap(
            divider.pt(adapter, maturity),
            Adapter(adapter).target(),
            ptBal,
            BalancerPool(spaceFactory.pools(adapter, maturity)).getPoolId(),
            0,
            payable(address(this))
        );
    }
}

Adapters can have their redeem restricted meaning the even when they are mature they can't be redeemed. In the scenario that it is restricted Periphery#_swapPTsForTarget simply won't work.

Impact

Redemption will fail when redeem is restricted because it tries to redeem instead of swapping

Code Snippet

Tool used

Manual Review

Recommendation

Use the same structure as _removeLiquidity:

    if (divider.mscale(adapter, maturity) > 0) {
        if (uint256(Adapter(adapter).level()).redeemRestricted()) {
            ptBal = _ptBal;
        } else {
            // 2. Redeem PTs for Target
            tBal += divider.redeem(adapter, maturity, _ptBal);
        }
jparklev commented 1 year ago

Accepted: This is valid and is indeed something we should fix

jparklev commented 1 year ago

Fixed here: https://github.com/sense-finance/sense-v1/pull/352

We removed the redeem code path if redeem is disabled

IAm0x52 commented 1 year ago

Fix looks good. Periphery will swap instead of redeeming if redeem is restricted