Unchecked External Call in deposit Function

Summary

The deposit function within the contract performs an unchecked external call to transferBatchItemsAcrossCollections function of the transferManager. This external call lacks proper checks and validation, potentially allowing reentrancy attacks or other unexpected behaviors.

Vulnerability Detail

In the deposit function, there is a loop where transferBatchItemsAcrossCollections is called for each deposit. However, the return value of this function is not checked, and there are no safeguards against reentrancy. If the transferManager contract behaves maliciously or unexpectedly, it could lead to loss of funds or other critical issues.

Impact

If the transferManager contract is compromised or contains vulnerabilities, it could lead to loss of user funds or the ability to manipulate contract state in an unauthorized manner.

Code Snippet

https://github.com/sherlock-audit/2024-01-looksrare/blob/main/contracts-yolo/contracts/YoloV2.sol#L1218

Tool used

Manual Review

Recommendation

Implement checks and validation for the return value of the external call to transferBatchItemsAcrossCollections. Consider using the Checks-Effects-Interactions pattern to prevent reentrancy issues. Additionally, ensure that transferManager is a trusted contract and consider implementing a mechanism to upgrade or replace it if vulnerabilities are discovered in the future.

Suggested Fix

for (uint256 i; i < deposits.length; ++i) {
    // ...
    bool transferSuccess = transferManager.transferBatchItemsAcrossCollections(batchTransferItems, msg.sender, address(this));
    require(transferSuccess, "TransferManager: transfer failed");
    // ...
}

Add appropriate error handling or revert messages to inform users about the failure and ensure that the contract state remains consistent and secure.

sherlock-audit / 2024-01-looksrare-judging

Anubis - Unchecked External Call in deposit Function #120