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

Incorrect Implimnetation Of `InheritanceManager::nonReentrant` Modifier Leaves The Contracts Functions Vulnerable To Reentrancy Vulnerability

kvltbyte

Summary

In the InheritanceManager::nonReentrantmodifier, the transient storage key that is read and he transient storage that is loaded to are different.

Vulnerability Details

The modifier InheritanceManager::nonReentrantis used to implement a reentrancy guard using transient storage. The idea is that if the current transient storage slot value is 1, then this means the call is reentrant, and the function should execute; otherwise, the call can proceed.

However, in the current implementation, the modifier reads from transient storage key 1 but updates storage key 0. This inconsistency. Since transient storage key 1 is never updated, its value will always be zero; hence, if there are any reentrant calls, this modifier will not prevent them.

modifier nonReentrant() {
assembly {
@> if tload(1) { revert(0, 0) } //reading key 1
tstore(0, 1) // updating key 0
}
_;
assembly {
tstore(0, 0) // updating key 0
}
}

Impact

  1. The modifier does to prevent any reentrancy vulnerabilities.

Tools Used

  1. Manual review.

Recommendations

Write to and read from the same transient storage key:

modifier nonReentrant() {
assembly {
- if tload(1) { revert(0, 0) }
+ if tload(0) { revert(0, 0) }
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}
Updates

Lead Judging Commences

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

Wrong value in nonReentrant modifier

0xtimefliez Lead Judge 5 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.