Inheritable Smart Contract Wallet

Summary

The nonReentrant modifier in the InheritanceManager contract uses different transient storage slots for checking and setting the reentrancy lock, making the implementation incorrect. However, due to the presence of onlyOwner modifier on all functions using this guard, no actual exploitation is possible.

Vulnerability Details

The nonReentrant modifier implementation uses slot 1 for checking the lock but slot 0 for setting it:

The modifier is used in the following functions:

• sendERC20

• sendETH

• contractInteractions

This means:

1. The check looks at slot 1 which is never modified

2. The lock is set in slot 0 which is never checked

3. The reentrancy protection is technically ineffective

However, all functions using this modifier (sendERC20 and sendETH) also implement the onlyOwner modifier, which prevents any potential reentrancy by ensuring only the contract owner can call these functions.

Impact

LOW

While the implementation of the reentrancy guard is incorrect, the actual security impact is minimal because:

1. No functions in the current implementation can be exploited due to the onlyOwner modifier

2. Any reentrancy attempt would fail at the onlyOwner check since the reentrant call would come from a different address

3. No loss of funds is possible with the current implementation

However, this could become a more serious issue if:

• The contract is inherited and the broken modifier is relied upon for security

• New functions are added that use only nonReentrant for protection

• The contract is modified to use nonReentrant without onlyOwner

Tools Used

• Manual review

• Code inspection

• Official Solidity Documentation Transient Storage Opcodes in Solidity

• Foundry tests

Recommendations

1. Update the nonReentrant modifier to use the same storage slot for both checking and setting the lock:

1. Consider using OpenZeppelin's battle-tested implementations instead of custom access control and security mechanisms:

This would provide several benefits:

• Use of thoroughly audited and widely deployed code

• Reduced risk of implementation errors

• Better standardization and maintainability

• Regular security updates and improvements from the OpenZeppelin team

• Increased trust from users and auditors who are familiar with these standard implementations