The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

`SmartVaultManagerV5` DOS of NFT transfers due to `SmartVaultIndex::transferTokenId` implementation if a user transfer empty vaults to receipients

carlitox477

Description

Whenever a vault is transfered, next function is triggered

function _afterTokenTransfer(address _from, address _to, uint256 _tokenId, uint256) internal override {
// @audit next line enable DOS
smartVaultIndex.transferTokenId(_from, _to, _tokenId);
​
// If from = address(0) can be a minting operation
// Change ownership
if (address(_from) != address(0)) ISmartVault(smartVaultIndex.getVaultAddress(_tokenId)).setOwner(_to);
emit VaultTransferred(_tokenId, _from, _to);
}

As we can see smartVaultIndex.transferTokenId(_from, _to, _tokenId); is triggered

function transferTokenId(address _from, address _to, uint256 _tokenId) external onlyManager {
// @audit this line enable DOS
removeTokenId(_from, _tokenId);
tokenIds[_to].push(_tokenId);
}

Then

// SmartVaultIndex.sol
function removeTokenId(address _user, uint256 _tokenId) private {
uint256[] memory currentIds = tokenIds[_user];
uint256 idsLength = currentIds.length;
delete tokenIds[_user];
for (uint256 i = 0; i < idsLength; i++) {
if (currentIds[i] != _tokenId) tokenIds[_user].push(currentIds[i]);
}
}

This last function deletes tokenIds[_user], here _user = _from. Therefore vaults transfers of address _from can be DOS if _from has enough vaults by transfering many empty vaults to force SmartVaultIndex::removeTokenId to consume gas over block gas limit through for loop

Impact

Anyone can DOS smart vault transfers by transfering empty vaults to the user to DOS

POC

  1. Alice has 3 vaults and want to transfer 1 to Bob

  2. Eve see these and mint 30.000 empty vaults. Each time he mint a vault he transfer it to Alice

  3. When Alice try to tranfer her vault to Bob, the transaction revert because block gas limit is reached

Recommended mitigation

Make more sense to make SmartVault an ERC721 NFT. To get the every vault from a user a mapping(address user => uint256[]tokenId) can be added, and modified every time a vault is minted or transfered.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

vault-dos

Support

FAQs

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

Give us feedback!