code-423n4 / 2024-01-renft-findings

2 stars 0 forks source link

Batch Transfers can lead to unauthorized transfer of rented assets #518

Closed c4-bot-2 closed 10 months ago

c4-bot-2 commented 10 months ago

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L280

Vulnerability details

if (selector == e1155_safe_batch_transfer_from_selector) {
    revert Errors.GuardPolicy_UnauthorizedSelector(
        e1155_safe_batch_transfer_from_selector
    );
}

Mitigation

// Implement a check for batch transfers that iterates through the token IDs and verifies none are rented.
function _validateBatchTransfer(bytes memory data) private view {
    // Decode the calldata to extract the token IDs array
    (, , uint256[] memory tokenIds) = abi.decode(data[4:], (address, address, uint256[]));
    for (uint256 i = 0; i < tokenIds.length; i++) {
        require(!STORE.isRentedOut(msg.sender, to, tokenIds[i]), "GuardPolicy: Token is rented");
    }
}

// Use the new validation function in the check
if (selector == e1155_safe_batch_transfer_from_selector) {
    _validateBatchTransfer(data);
}

Impact

The impact of allowing a batch transfer of rented ERC1155 tokens is that it could violate the terms of the rental agreement and potentially result in financial loss or reputational damage to the rental platform. The mitigation code introduces a check that iterates through the token IDs in a batch transfer to ensure that none of them are currently rented out. This preserves the integrity of the rental agreements and prevents unauthorized transfers of rented assets.

Assessed type

Access Control

c4-pre-sort commented 10 months ago

141345 marked the issue as insufficient quality report

c4-judge commented 10 months ago

0xean marked the issue as unsatisfactory: Insufficient quality