Summary
The removeBeneficiary function doesn't update the inactivity deadline, potentially allowing premature inheritance.
Vulnerability Details
Affected Code - https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/main/src/InheritanceManager.sol#L163-L166
https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L143C4-L147C6
https://github.com/CodeHawks-Contests/2025-03-inheritable-smart-contract-wallet/blob/9de6350f3b78be35a987e972a1362e26d8d5817d/src/InheritanceManager.sol#L120C4-L130C6
function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
}
Unlike most owner functions, removeBeneficiary doesn't call _setDeadline(), Which breaks the core invariant as per specified in readme.
EVERY transaction the owner does with this contract must reset the 90 days timer
This inconsistent behavior means the inactivity timer isn't reset when removing beneficiaries.
This function has another issue too, submitted in another finding in detail
other affected functions are contractInteractions and createEstateNFT
POC
Add following test to existing test -
function testDeadlineIsNotUpdatedWithEveryOwnerAction() public {
vm.prank(owner);
im.addBeneficiery(user1);
uint256 afterAdd = im.getDeadline();
vm.warp(block.timestamp + 30 days);
uint256 beforeRemove = im.getDeadline();
vm.prank(owner);
im.removeBeneficiary(user1);
uint256 afterRemove = im.getDeadline();
assertEq(
beforeRemove,
afterRemove,
"Deadline shouldn't change but should"
);
console.log("Deadline after add:", afterAdd);
console.log("Deadline after remove:", afterRemove);
}
when running forge test --mt testDeadlineIsNotUpdatedWithEveryOwnerAction -vv we get following output, which confirms our finding.
Ran 1 test for test/InheritanceManagerTest.t.sol:InheritanceManagerTest
[PASS] testDeadlineIsNotUpdatedWithEveryOwnerAction() (gas: 75017)
Logs:
Deadline after add: 7776001
Deadline after remove: 7776001
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.45ms (1.11ms CPU time)
Ran 1 test suite in 161.34ms (7.45ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
This inconsistency could lead to premature inheritance if an owner removes beneficiaries and then becomes inactive for a period shorter than the intended timelock.
Tools Used
Foundry
Recommendations
update the functions like this to fix the issue
function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
beneficiaries[indexToRemove] = beneficiaries[beneficiaries.length - 1];
beneficiaries.pop();
_setDeadline();
}
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();
}
