Description:
Some functions do not call _setDeadline() when they should. This breaks a core assumption of the protocol:
"EVERY transaction the owner makes with this contract must reset the 90-day timer."
If an owner interacts with the contract using these specific functions, the deadline does not reset. This could lead to a scenario where the inherit() function is called prematurely, even though the owner was recently active.
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 removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
}
function createEstateNFT(
string memory _description,
uint256 _value,
address _asset
) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
assetToPay = _asset;
}
Impact:
The likelihood will be high as If the owner only interacts via these functions, the deadline does not reset.
The imapct is high as it will lead to Premature inheritance execution breaking the main Invariant and the porpuse of the protocol. It could lead to an unexpected loose of the ownership of the contract,
Proof of Concept:
PoC
function test_audit_BreakInvariant_No_setDeadline() public {
vm.deal(address(im), 10e18);
vm.startPrank(owner);
uint256 deadLine = im.getDeadline();
console.log(deadLine);
im.addBeneficiery(address(owner));
im.addBeneficiery(address(user1));
im.addBeneficiery(address(user2));
uint256 deadLine2 = im.getDeadline();
console.log(deadLine2);
vm.warp(deadLine2 + 90 days);
bytes memory call = abi.encodeWithSignature(
"createEstate(string)",
"pepe"
);
im.contractInteractions(address(nft), call, 0, false);
uint256 deadLine3 = im.getDeadline();
console.log(deadLine3);
vm.stopPrank();
vm.startPrank(user1);
im.inherit();
console.log("IsInherited is now: ", im.getIsInherited());
vm.stopPrank();
}
Recommended Mitigation:
To avoid this issue, you need to include InheritanceManager::_setDeadline() to every method where the owner interacts with the contract.
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 removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
+ _setDeadline();
}
function createEstateNFT(
string memory _description,
uint256 _value,
address _asset
) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
assetToPay = _asset;
+ _setDeadline();
}
