Using wrong selector will dos FlashRolloverLoan's accept commitment

Summary

The _acceptCommitment function will always fail if _commitmentArgs.smartCommitmentAddress != address(0) because the call forwarded uses a wrong selector.

Vulnerability Detail

The FlashRolloverLoan_G5 contract allows calling the smartcommitmentforwarder’s acceptCommitmentWithRecipient via its internal _acceptCommitment function:

// FlashRolloverLoan_G5.sol
function _acceptCommitment(
        address lenderCommitmentForwarder,
        address borrower,
        address principalToken,
        AcceptCommitmentArgs memory _commitmentArgs
    )
        internal
        virtual
        returns (uint256 bidId_, uint256 acceptCommitmentAmount_)
    {
        uint256 fundsBeforeAcceptCommitment = IERC20Upgradeable(principalToken)
            .balanceOf(address(this));

        if (_commitmentArgs.smartCommitmentAddress != address(0)) {

             bytes memory responseData = address(lenderCommitmentForwarder)
                    .functionCall(
                        abi.encodePacked(
                            abi.encodeWithSelector(
                                ISmartCommitmentForwarder
                                    .acceptSmartCommitmentWithRecipient 
                                    .selector, 
                                _commitmentArgs.smartCommitmentAddress,
                                _commitmentArgs.principalAmount,
                                _commitmentArgs.collateralAmount,
                                _commitmentArgs.collateralTokenId,
                                _commitmentArgs.collateralTokenAddress,
                                address(this),
                                _commitmentArgs.interestRate,
                                _commitmentArgs.loanDuration
                            ),
                            borrower //cant be msg.sender because of the flash flow
                        )
                    );

                (bidId_) = abi.decode(responseData, (uint256));

        } else { 

        ...

The problem is that the call to the smart commitment forwarded is performed with the ISmartCommitmentForwarder.acceptSmartCommitmentWithRecipient.selector, given by the ISmartCommitmentForwarder interface:

// ISmartCommitmentForwarder.sol

function acceptSmartCommitmentWithRecipient(
        address _smartCommitmentAddress,
        uint256 _principalAmount,
        uint256 _collateralAmount,
        uint256 _collateralTokenId,
        address _collateralTokenAddress,
        address _recipient,
        uint16 _interestRate,
        uint32 _loanDuration
    ) external  returns (uint256 bidId);

This is wrong because the smart commitment forward contract doesn’t have a acceptSmartCommitmentWithRecipient. Instead, it only has a acceptCommitmentWithRecipient function. Because of this, all the calls made to the smart commitment forwarder will fail, given that no function with the selector given by the acceptSmartCommitmentWithRecipient will be found.

Impact

Medium. The functionality allowing to interact with the smart commitment forwarder in the FlashRolloverLoan_G5 contract will never work and always revert.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/FlashRolloverLoan_G5.sol#L291-L293

Tool used

Manual Review

Recommendation

Update the interface and call used so that the function can be properly forwarded.

// ISmartCommitmentForwarder.sol

pragma solidity >=0.8.0 <0.9.0;

interface ISmartCommitmentForwarder {

-    function acceptSmartCommitmentWithRecipient(
+ function acceptCommitmentWithRecipient(
        address _smartCommitmentAddress,
        uint256 _principalAmount,
        uint256 _collateralAmount,
        uint256 _collateralTokenId,
        address _collateralTokenAddress,
        address _recipient,
        uint16 _interestRate,
        uint32 _loanDuration
    ) external  returns (uint256 bidId)  ;

}

// FlashRolloverLoan_G5.sol

function _acceptCommitment(
        address lenderCommitmentForwarder,
        address borrower,
        address principalToken,
        AcceptCommitmentArgs memory _commitmentArgs
    )
        internal
        virtual
        returns (uint256 bidId_, uint256 acceptCommitmentAmount_)
    {
        uint256 fundsBeforeAcceptCommitment = IERC20Upgradeable(principalToken)
            .balanceOf(address(this));

        if (_commitmentArgs.smartCommitmentAddress != address(0)) {

             bytes memory responseData = address(lenderCommitmentForwarder)
                    .functionCall(
                        abi.encodePacked(
                            abi.encodeWithSelector(
                                ISmartCommitmentForwarder
-                                    .acceptSmartCommitmentWithRecipient 
+                                    .acceptCommitmentWithRecipient 
                                    .selector, 
                                _commitmentArgs.smartCommitmentAddress,
                                _commitmentArgs.principalAmount,
                                _commitmentArgs.collateralAmount,
                                _commitmentArgs.collateralTokenId,
                                _commitmentArgs.collateralTokenAddress,
                                address(this),
                                _commitmentArgs.interestRate,
                                _commitmentArgs.loanDuration
                            ),
                            borrower //cant be msg.sender because of the flash flow
                        )
                    );

                (bidId_) = abi.decode(responseData, (uint256));

        } else { 

        ...

Duplicate of #135

sherlock-audit / 2024-04-teller-finance-judging

0xadrii - Using wrong selector will dos FlashRolloverLoan's accept commitment #246