Bauer - User will lose the target token

[sherlock-admin]

Bauer

high

User will lose the target token

Summary

Users can burn PT and YT to get target token. However, if the quote.sellToken is the same as quote.buyToken, but not the same as underly token and target token ,then there is no swap on 0x exchange, the _fillQuote return 0 amount of quote.buyToken. The protocol only transfer the buy token to the user, not the target token. User will lose the target token.

Vulnerability Detail

The combine() is used to reconstitute Target by burning PT and YT. The protocol first transfer PTs and YTs tokens from msg.sender to Perphery contract. Then, call the Divider.combine() to reconstitute target by burning PT and YT. Next, try to swap quote.sellToken to quote.buyToken on 0x exchange.The quote.sellToken and quote.buyToken parameter are specified by the user. However ,if the quote.sellToken is the same as quote.buyToken, but not the same as underly token and target token , then there is no swap on 0x exchange.User will lose the target token.

   function combine(
        address adapter,
        uint256 maturity,
        uint256 uBal,
        address receiver,
        PermitBatchData calldata permit,
        SwapQuote calldata quote
    ) external returns (uint256 amt) {
        IPermit2.SignatureTransferDetails[] memory sigs = new IPermit2.SignatureTransferDetails[](2);
        sigs[0] = IPermit2.SignatureTransferDetails({ to: address(this), requestedAmount: uBal });
        sigs[1] = IPermit2.SignatureTransferDetails({ to: address(this), requestedAmount: uBal });

        // pull underlying
        permit2.permitTransferFrom(permit.msg, sigs, msg.sender, permit.sig);
        amt = _fromTarget(adapter, divider.combine(adapter, maturity, uBal), quote);
        _transfer(quote.buyToken, receiver, amt);
    }

function _fillQuote(SwapQuote calldata quote) internal returns (uint256 boughtAmount) {
        if (quote.sellToken == quote.buyToken) return 0; // No swap if the tokens are the same.
        if (quote.swapTarget != exchangeProxy) revert Errors.InvalidExchangeProxy();

Impact

User will lose the target token

Code Snippet

https://github.com/sherlock-audit/2023-03-sense/blob/main/sense-v1/pkg/core/src/Periphery.sol#L433-L449 https://github.com/sherlock-audit/2023-03-sense/blob/main/sense-v1/pkg/core/src/Periphery.sol#L903

Tool used

Manual Review

Recommendation

Inside the combine() function ,transfer target token to users.

Duplicate of #29

[jparklev]

Dispute severity: based on the impact criteria, we believe this is a Medium severity issue! While it is a user input error, and we do put some responsibility on them to get the quote details right if they're setting up the tx manually, it's still an unfortunately state

We appreciate the call out

[IAm0x52]

Duplicate of #29. One of the many occurrences listed of this same issue

From #29: Periphery#combine may leave excess underlying in the contract due to _fromTarget unwrapping to underlying and the quote may not swap them all.