Avci - If transferFrom() fails, user can mint _setToken for free.

[sherlock-admin]

Avci

high

If transferFrom() fails, user can mint _setToken for free.

Summary

Use safeTransferFrom instead of transferFrom

Vulnerability Detail

The transferFrom() functions will return a boolean value to let you know whether the transfer was successful or not. It's important to check this value to ensure that the transfer went through as intended. And for certain tokens, instead of reverting the transaction in case of a failed transfer, simply returns a false value.

Impact

If transferFrom() fails, user can mint _setToken for free.

Code Snippet

function issue(
        ISetToken _setToken,
        uint256 _quantity,
        address _to
    )
        external
        nonReentrant
        onlyValidAndInitializedSet(_setToken)
    {
        require(_quantity > 0, "Issue quantity must be > 0");

        address hookContract = _callPreIssueHooks(_setToken, _quantity, msg.sender, _to);

        (
            address[] memory components,
            uint256[] memory componentQuantities
        ) = getRequiredComponentUnitsForIssue(_setToken, _quantity);

        // For each position, transfer the required underlying to the SetToken
        for (uint256 i = 0; i < components.length; i++) {
            // Transfer the component to the SetToken
            transferFrom(
                IERC20(components[i]),
                msg.sender,
                address(_setToken),
                componentQuantities[i]
            );  //@audit Consider using safeTransferFrom
        }   //@audit Some tokens like USDT/USDC doesnt work with IERC20 standard

        // Mint the SetToken
        _setToken.mint(_to, _quantity);

        emit SetTokenIssued(address(_setToken), msg.sender, _to, hookContract, _quantity);
    }

https://github.com/sherlock-audit/2023-06-Index/blob/8d348ed344635a068d458aa04956f966b6d3d4f3/index-protocol/contracts/protocol/modules/v1/BasicIssuanceModule.sol#L107-L112

Tool used

Manual Review

Recommendation

Consider Using safeTransferFrom. instead of transferFrom