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

2 stars 0 forks source link

Rental Token Transfer Attempt #515

Closed c4-bot-5 closed 10 months ago

c4-bot-5 commented 10 months ago

Lines of code

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

Vulnerability details

if (selector == e721_safe_transfer_from_1_selector) {
    // Load the token ID from calldata.
    uint256 tokenId = uint256(
        _loadValueFromCalldata(data, e721_safe_transfer_from_1_token_id_offset)
    );

    // Check if the selector is allowed.
    _revertSelectorOnActiveRental(selector, from, to, tokenId);
}
// Similar checks for other ERC721 and ERC1155 transfer selectors...

Mitigation

// Implement a more robust method of parsing calldata that does not rely on fixed offsets.
function _parseTokenId(bytes memory data, bytes4 selector) private pure returns (uint256 tokenId) {
    // Check the selector and parse accordingly
    if (selector == e721_safe_transfer_from_1_selector) {
        tokenId = abi.decode(data[36:], (uint256)); // Assuming standard ERC721 layout
    } else if (selector == e721_safe_transfer_from_2_selector) {
        tokenId = abi.decode(data[68:], (uint256)); // Assuming standard ERC721 layout
    }
    // Add more conditions for other selectors if necessary
    return tokenId;
}

// Use the new parsing function in the check
if (selector == e721_safe_transfer_from_1_selector || selector == e721_safe_transfer_from_2_selector) {
    uint256 tokenId = _parseTokenId(data, selector);
    _revertSelectorOnActiveRental(selector, from, to, tokenId);
}

Impact

The impact of incorrectly parsing the token ID is that a rented token could be transferred without authorization, violating the rental agreement and potentially leading to financial loss for the renter or the rental platform. The mitigation code introduces a more flexible and reliable way to parse the token ID from the calldata, reducing the risk of incorrect token ID extraction.

Assessed type

Token-Transfer

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