Inheritable Smart Contract Wallet

Summary

The nonReentrant modifier intends to protect against reentrancy by using transient storage. However, it incorrectly checks one storage slot (slot 1) while setting and later clearing another (slot 0), rendering the reentrancy guard ineffective.

Vulnerability Details

The modifier is written as follows:

The intended mechanism is to check if a reentrant call is in progress by reading a flag from transient storage. However, the code reads from slot 1 using tload(1) while setting the flag in slot 0 using tstore(0, 1). As slot 1 is never updated, the check always passes, leaving the contract vulnerable to reentrancy in complex call scenarios.

Impact

• Ineffective Reentrancy Protection: The reentrancy guard fails to prevent nested calls, potentially exposing the contract to reentrancy attacks.

• Increased Attack Surface: An attacker could exploit the vulnerability to manipulate state changes or drain funds.

• Unexpected Contract Behavior: The incorrect use of transient storage can lead to unpredictable behavior in multi-call transactions, undermining composability.

Tools Used

• Manual Code Review

Recommendations

1. Correct the Storage Slot Usage:
   Use the same transient storage slot for both the check and the assignment:

   modifier nonReentrant() {

   assembly {

   if tload(0) { revert(0, 0) }

   tstore(0, 1)

   }

   _;

   assembly {

   tstore(0, 0)

   }

   }

   ​

2. Review Transient Storage Use:
   Ensure that transient storage is cleared at the end of each call to maintain composability, as per EIP-1153 guidelines.

3. Conduct Thorough Testing:
   Perform extensive tests, including simulating reentrancy attacks, to confirm that the modified guard effectively prevents reentrant calls.

4. Consider Established Patterns:
   Evaluate using well-audited reentrancy guard patterns, such as those provided by OpenZeppelin, to avoid similar issues in the future.