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

NonReentrant modifier doesn't function as it should, causing reentrancy vulnerability in functions

uddercover

Summary

The NonReentrant modifier in InheritanceManager.sol attempts to use transient storage to prevent reentrancy, but it is not implemented correctly causing the modifier to be useless.

Vulnerability Details

modifier nonReentrant() {
assembly {
@> if tload(1) { revert(0, 0) }
@> tstore(0, 1)
}
//...
}

The first noted line checks that the value at key "1" is false before calling tstore but tstore stores a value at key "0" instead. Meaning that tload(1) would always return false even when the call is reentrant, allowing the call to proceed.

POC

Add the following test into the InheritanceManagerTest.t.sol

function testNonReentrantDoesntWork() public {
Reenter reenter = new Reenter();
uint256 sendAmount = 1 ether;
​
deal(address(im), 10 ether);
​
vm.startPrank(owner);
//reverts internally in reentrant call with "not owner" error from the onlyOwner modifier which comes after the nonreentrant modifier meaning the nonreentrant lock gets bypassed
im.sendETH(sendAmount, address(reenter));
vm.stopPrank();
}
​
​
contract Reenter {
fallback() external payable {
bytes memory data = abi.encodeWithSignature("sendETH(uint256,address)", msg.value, address(this));
if (msg.sender.balance > msg.value) {
(msg.sender).call(data);
}
}
}

Impact

The nonreentrant modifier is useless, allowing reentrant calls into sensitive functions

Tools Used

manual review, foundry

Recommendations

modifier nonReentrant() {
assembly {
+ if tload(0) { revert(0, 0) }
- if tload(1) { 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

Support

FAQs

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