Inheritable Smart Contract Wallet
First Flight #35
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

tload(1) should be tload(0) or it won't avoid reentrancy

0xziin

Summary

The wrong variable is read in the nonReentrant modifier's code resulting in a reentrancy attack vector.

Vulnerability Details

https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/main/src/InheritanceManager.sol#L70

In InheritanceManager.sol, tload(1) is used to check if we already entered the function instead of tload(0) that is actually used afterwards :

/**
* @dev gas efficient cross-function reentrancy lock using transient storage
* @notice refer here: https://soliditylang.org/blog/2024/01/26/transient-storage/
*/
modifier nonReentrant() {
assembly {
if tload(1) { revert(0, 0) } // <- here it should be tload(0) instead of tload(1)
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}

tstore(0,1) and tstore(0,0) are using the "0" transient variable not the "1", it means we should be reading the "0" transient variable with tload(0) instead of tload(1) for the code to function as intended.

Impact

The nonReentrant modifier won't work as intended. It won't avoid reentering the functions.
==> It could lead to reentrancy attacks like draining funds from the contract.

Tools Used

Manual review, Github.

Recommendations

Replace tload(1) with tload(0).

Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong value in nonReentrant modifier

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong value in nonReentrant modifier

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.