Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

DoS Risk with Safe Transfers

fromeo_016

Summary

If safeTransferFrom is used, and the recipient contract maliciously rejects the transfer (by not implementing the ERC721Receiver interface or intentionally reverting the transfer), a denial-of-service (DoS) attack could be created. In such cases, no tokens can be transferred successfully, affecting the entire system.

Vulnerability Details

The issue occurs when using safeTransferFrom for NFT transfers:

raacNFT.safeTransferFrom(address(this), msg.sender, tokenId);

The vulnerability arises because:

safeTransferFrom checks if recipient is a contract
If so, it calls onERC721Received on the recipient
Malicious contracts can:

Not implement ERC721Receiver interface
Implement but revert the callback
Implement but consume excessive gas

This creates a situation where transfers can be deliberately blocked by malicious recipient contracts.

Impact

System operations requiring NFT transfers become unusable, leading to a denial-of-service (DoS) attack.

Tools Used

Manual code review

Recommendations

Consider implementing error handling to prevent the system from getting stuck.

/**
* @notice Allows a user to withdraw an NFT
* @param tokenId The token ID of the NFT to withdraw
*/
function withdrawNFT(uint256 tokenId) external nonReentrant whenNotPaused {
if (isUnderLiquidation[msg.sender]) revert CannotWithdrawUnderLiquidation();
​
UserData storage user = userData[msg.sender];
if (!user.depositedNFTs[tokenId]) revert NFTNotDeposited();
​
// update state
ReserveLibrary.updateReserveState(reserve, rateData);
​
// Check if withdrawal would leave user undercollateralized
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
uint256 collateralValue = getUserCollateralValue(msg.sender);
uint256 nftValue = getNFTPrice(tokenId);
​
if (collateralValue - nftValue < userDebt.percentMul(liquidationThreshold)) {
revert WithdrawalWouldLeaveUserUnderCollateralized();
}
​
// Remove NFT from user's deposited NFTs
for (uint256 i = 0; i < user.nftTokenIds.length; i++) {
if (user.nftTokenIds[i] == tokenId) {
user.nftTokenIds[i] = user.nftTokenIds[user.nftTokenIds.length - 1];
user.nftTokenIds.pop();
break;
}
}
user.depositedNFTs[tokenId] = false;
​
- raacNFT.safeTransferFrom(address(this), msg.sender, tokenId);
+ try raacNFT.safeTransferFrom(address(this), winner, tokenId) returns(bytes memory) {
+ // Transfer was successful
+ } catch (bytes memory) {
+ // Handle any error
+ revert("ERC721 transfer failed");
+ }
emit NFTWithdrawn(msg.sender, tokenId);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.