sherlock-audit / 2023-06-Index-judging

6 stars 2 forks source link

Oxhunter526 - No revert on Failure on Erc20 Token Transfer #59

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Oxhunter526

high

No revert on Failure on Erc20 Token Transfer

Summary

The code in the _executeBid function lacks proper error handling when invoking the transferFrom function. This omission poses a risk as it doesn't validate the success of the token transfer, potentially leading to unexpected behavior or loss of funds.

Vulnerability Detail

The _executeBid function transfers tokens from the _bidInfo.receiveToken address to the msg.sender and then invokes a transfer from the SetToken contract to the msg.sender. However, it fails to check the return value of the transferFrom function, disregarding any potential transfer failures.

Impact

If a transfer fails, the code will continue executing, leading to unexpected behavior or loss of funds for the users involved in the bid process.

Code Snippet

Code Snippet01 Code Snippet02

Tool used

Manual Review

Recommendation

Utilize the require statement or a similar mechanism, so that the code can validate the success of the transfer and revert the transaction if it fails. Here's an example of how to mitigate the vulnerability:

function _executeBid(
    BidInfo memory _bidInfo
)
    internal
{
    // Transfer the received tokens from the sender to the SetToken.
    require(
        transferFrom(
            _bidInfo.receiveToken,
            msg.sender,
            address(_bidInfo.setToken),
            _bidInfo.quantityReceivedBySet
        ),
        "Transfer from sender to SetToken failed"
    );

    // Invoke the transfer of the sent tokens from the SetToken to the sender.
    require(
        _bidInfo.setToken.strictInvokeTransfer(
            address(_bidInfo.sendToken),
            msg.sender,
            _bidInfo.quantitySentBySet
        ),
        "Transfer from SetToken to sender failed"
    );
}