Inheritable Smart Contract Wallet

Summary

The buyOutStateNft function in the InheritanceManager contract contains a logic flaw where the return statement inside the for loop causes premature function termination. If msg.sender is found in the beneficiaries list, the function exits early, preventing the remaining beneficiaries from receiving their rightful share and stopping the NFT from being burned.

Vulnerability Details

The function is created to allow a beneficiary to buy out an estate NFT by paying other beneficiaries their proportional share. However, the issue lies in the for loop:

If msg.sender is found in beneficiaries, the return statement causes the function to exit immediately. There are three possible cases:

1. If msg.sender is in the beneficiaries[first_position], then any of the beneficiaries will not get their share and estate Nft isn't burned.

2. If msg.sender is in somewhere between first and last_position, then some of the beneficiaries will not get their share and estate Nft isn't burned.

3. And if msg.sender is in the beneficiaries[last_position], then only in this case everyone will get their share and estate Nft is burned.

Impact

Funds Loss & Unfair Distribution: The intended beneficiaries may not receive their rightful payments.

Asset Locking: The NFT remains in circulation despite the intent to burn it.

Potential Exploitation: A malicious beneficiary could exploit this by making themselves msg.sender, causing the function to exit early and preventing beneficiaries listed after them in the array from receiving their rightful share.

Tools Used

Manual review

Recommendations

Use continue statement instead of return. Or use the more optimized method given below-