Inheritable Smart Contract Wallet
First Flight #35
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Premature Return In `InheritanceManager::buyOutEstateNFT()` Leading To Partial and Incorrect Distributon Of Assets.

kvltbyte

Summary

The InheritanceManager::buyOutEstateNFT()function returns prematurely when msg.sender == beneficiaries[i]) even before it loops through the entire array.

Vulnerability Details

TheInheritanceManager::buyOutEstateNFT() function enables one beneficiary to buy out the others by distributing the value of the NFT to the others and burning the underlying NFT.

This is done by looping through the array of beneficiaries and sending them their shares one by one. If the current index of he beneficiaries' array points to the user's address buying out the others, then it should skip sending them this share since the user buying out the others should not receive a share.

However, in the current implementation, the function will return prematurely when this condition is met, thereby skipping all subsequent beneficiaries that come after the msg.sender in the array.

for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
@> return;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}

Impact

  1. Incomplete distribution of assets.

  2. Failure to burn the underlying NFT.

Tools Used

  1. Manual review.

Recommendations

  1. Modify the code to use continueinstead of return

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
for (uint256 i = 0; i < beneficiaries.length; i++) {
if (msg.sender == beneficiaries[i]) {
- return;
+ continue;
} else {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}

  1. Conditionally send when assets when the current address isn't the msg.sender:

function buyOutEstateNFT(uint256 _nftID) external onlyBeneficiaryWithIsInherited {
uint256 value = nftValue[_nftID];
uint256 divisor = beneficiaries.length;
uint256 multiplier = beneficiaries.length - 1;
uint256 finalAmount = (value / divisor) * multiplier;
​
IERC20(assetToPay).safeTransferFrom(msg.sender, address(this), finalAmount);
​
for (uint256 i = 0; i < beneficiaries.length; i++) {
- if (msg.sender == beneficiaries[i]) {
- return;
- } else {
+ if (msg.sender != beneficiaries[i]) {
IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
​
nft.burnEstate(_nftID);
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge 5 months ago
Submission Judgement Published
Validated
Assigned finding tags:

buyOutNFT has return instead of continue

Support

FAQs

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