Summary:
Hi,
I have found out some potentital vulnerability in the contract 'InheritanceManager.sol', in which the error of unsafe division in two functions: 'withdrawInheritedFunds' and 'buyOutEstateNFT' arises causing arithmetic error and token loss problem.
Vulnerability Details:
The key points of this potential vulnerability can be given as below:
In the contract 'InheritanceManager.sol', the functions 'withdrawInheritedFunds' and 'buyOutEstateNFT' are performing division operations i.e. amountPerBeneficiary = assetAmountAvailable / divisor without handling potential rounding issues as of line 240 in the contract uint256 divisor = beneficiaries.length; taking the account of no. of beneficiaries associated for inheriting the amount in the contract leads to token loss.
**For eg: **if assetAmountAvailable = 100 and divisor = 3 results 33 each beneficiary leaving 1 token stuck in contract.
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 divisor = beneficiaries.length;
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
@> uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
(bool success,) = beneficiary.call{value: amountPerBeneficiary}("");
require(success, "something went wrong");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
@> uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
for (uint256 i = 0; i < divisor; i++) {
IERC20(_asset).safeTransfer(beneficiaries[i], amountPerBeneficiary);
}
}
}
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 {
@> IERC20(assetToPay).safeTransfer(beneficiaries[i], finalAmount / divisor);
}
}
nft.burnEstate(_nftID);
}
Impact:
Small amounts of funds permanently locked in contract, leads to disputes amoung beneficiaries.
Tools Used:
Manual Code Analysis + VS Code
Recommendations:
Implement a mechanism to handle remainders, such as sending the leftover amount to a designated address (e.g., the first beneficiary) or ensuring the total amount is fully distributed. Here's an example fix for the function 'withdrawInheritedFunds':
function withdrawInheritedFunds(address _asset) external {
if (!isInherited) {
revert NotYetInherited();
}
uint256 divisor = beneficiaries.length;
require(divisor > 0, "No beneficiaries");
if (_asset == address(0)) {
uint256 ethAmountAvailable = address(this).balance;
uint256 amountPerBeneficiary = ethAmountAvailable / divisor;
uint256 remainder = ethAmountAvailable % divisor;
for (uint256 i = 0; i < divisor; i++) {
address payable beneficiary = payable(beneficiaries[i]);
uint256 amountToSend = amountPerBeneficiary;
if (i == 0) {
amountToSend += remainder;
}
(bool success,) = beneficiary.call{value: amountToSend}("");
require(success, "Transfer failed");
}
} else {
uint256 assetAmountAvailable = IERC20(_asset).balanceOf(address(this));
uint256 amountPerBeneficiary = assetAmountAvailable / divisor;
uint256 remainder = assetAmountAvailable % divisor;
for (uint256 i = 0; i < divisor; i++) {
uint256 amountToSend = amountPerBeneficiary;
if (i == 0) {
amountToSend += remainder;
}
IERC20(_asset).safeTransfer(beneficiaries[i], amountToSend);
}
}
}
