Inheritable Smart Contract Wallet

Description:

The InheritanceManager::nonReentrant modifier that's intended to protect against reentrancy attacks contains a critical vulnerability due to inconsistent transient storage slot usage. The modifier checks one storage slot but sets a different one:

This implementation checks the value in transient storage slot 1 (tload(1)), but it sets and resets the flag in transient storage slot 0 (tstore(0, 1) and tstore(0, 0)). As a result, the reentrancy check will never detect a reentrant call because it's monitoring a different slot than the one being modified.

Impact:

This vulnerability completely breaks the reentrancy protection mechanism.

However, since all functions where the nonReentrant modifier is implemented (InheritanceManager::sendERC20, InheritanceManager::sendETH, and InheritanceManager::contractInteractions) are also protected by the onlyOwner modifier, this issue is not a problem.

InheritanceManager::sendERC20 and InheritanceManager::sendETH send tokens from the contract. Since only the owner can do this, reentrancy is not applicable.

InheritanceManager::contractInteractions calls external contracts (which could be malicious), but the receive/fallback call will not go through because msg.sender will be the address of the malicious contract and not the owner, so it will revert.

It's a low / informational severity

Recommended Mitigation:

Fix the transient storage slot inconsistency by using the same slot for both checking and setting the reentrancy guard: