KupiaSec - `FlashRolloverLoan_G5` cannot work well with some LenderCommitForwarders including the `SmartCommitmentForwarder` contract.

[sherlock-admin2]

KupiaSec

medium

FlashRolloverLoan_G5 cannot work well with some LenderCommitForwarders including the SmartCommitmentForwarder contract.

Summary

The SmartCommitmentForwarder contract doesn't have the acceptSmartCommitmentWithRecipient() and getCommitmentMarketId() functions. However, FlashRolloverLoan_G5 calles the functions. So, FlashRolloverLoan_G5 cannot work with the SmartCommitmentForwarder contract.

Vulnerability Detail

FlashRolloverLoan_G5 calles the acceptSmartCommitmentWithRecipient() and getCommitmentMarketId() functions of LenderCommitForwarder.

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

    function _acceptCommitment(
        address lenderCommitmentForwarder,
        address borrower,
        address principalToken,
        AcceptCommitmentArgs memory _commitmentArgs
    )
        [...]
             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
                        )
                    );
        [...]
    }

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

    function _getMarketIdForCommitment(address _lenderCommitmentForwarder, uint256 _commitmentId)
        internal
        view
        returns (uint256)
    {
@>      return ILenderCommitmentForwarder(_lenderCommitmentForwarder).getCommitmentMarketId(_commitmentId);
    }

However, some LenderCommitForwarders don't have these functions. As a result, FlashRolloverLoan_G5 cannot work well with some LenderCommitForwarders including the SmartCommitmentForwarder contract.

Impact

FlashRolloverLoan_G5 cannot work with some LenderCommitForwarders including the SmartCommitmentForwarder contract.

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#L270-L368

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

Tool used

Manual Review

Recommendation

The getCommitmentMarketId function should be added into the SmartCommitmentForwarder contract.

And the function selector should be corrected as follows.

    function _acceptCommitment(
            [...]

        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
                        )
                    );
            [...]
    }

Duplicate of #135