Summary
Multiple owner-callable functions don't reset the inheritance timelock deadline, allowing premature inheritance even when the owner is actively using the contract. This directly violates the project's first core assumption: "EVERY transaction the owner does with this contract must reset the 90 days timer."
Vulnerability Details
The InheritanceManager contract uses a deadline-based system to determine when beneficiaries can inherit funds. This deadline should be extended whenever the owner interacts with the contract. However, three important owner functions fail to reset the deadline:
function contractInteractions(address _target, bytes calldata _payload, uint256 _value, bool _storeTarget)
external
nonReentrant
onlyOwner
{
(bool success, bytes memory data) = _target.call{value: _value}(_payload);
require(success, "interaction failed");
if (_storeTarget) {
interactions[_target] = data;
}
}
function createEstateNFT(string memory _description, uint256 _value, address _asset) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
assetToPay = _asset;
}
function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
}
This inconsistency means an owner could interact with their wallet regularly but still have their funds inherited by beneficiaries if they only use these specific functions.
Impact
The impact is severe as it undermines the core security model of the contract:
Beneficiaries can gain access to funds while the owner is still active
The owner could lose funds despite regular contract interaction
This breaks the stated invariant that all owner activity should reset the timelock
The proof of concept test demonstrates that even after multiple owner interactions, the inheritance process can still be triggered because the deadline was never updated.
Tools Used
Manual code review and Foundry testing framework
Proof of Concept:
Add the following test to InheritanceManager.t.sol
function test_inheritanceManagerInteractionsUpdateDeadline() public {
address user2 = makeAddr("user2")
vm.startPrank(owner);
im.addBeneficiery(user1);
im.addBeneficiery(user2);
vm.stopPrank();
vm.warp(90 days);
assertEq(1 + 90 days, im.getDeadline());
vm.prank(user1);
vm.expectRevert(InheritanceManager.InactivityPeriodNotLongEnough.selector);
im.inherit();
// Now call the functions that do not update the deadline
vm.startPrank(owner);
vm.warp(1 + 100 days);
im.contractInteractions(address(0), abi.encode(address(0)), 0, false);
vm.warp(1 + 120 days);
im.createEstateNFT("our beach-house", 2000000, address(usdc));
vm.stopPrank();
assertEq(1 + 90 days, im.getDeadline());
assertEq(vm.getBlockTimestamp(), 1 + 120 days);
vm.prank(user1);
im.inherit();
assertEq(true, im.getIsInherited());
}
run the test
forge test --mt test_inheritanceManagerInteractionsUpdateDeadline
Recommendations
Add the _setDeadline() function call to all owner-only functions to ensure consistent deadline updates:
function contractInteractions(address _target, bytes calldata _payload, uint256 _value, bool _storeTarget)
external
nonReentrant
onlyOwner
{
(bool success, bytes memory data) = _target.call{value: _value}(_payload);
require(success, "interaction failed");
if (_storeTarget) {
interactions[_target] = data;
}
+ _setDeadline();
}
function createEstateNFT(string memory _description, uint256 _value, address _asset) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
assetToPay = _asset;
+ _setDeadline();
}
function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
+ _setDeadline();
}
