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

Not all functions called by `onlyOwner` or the owner of `InheritanceManager` reset the deadline which breaks the criteria for being inactive.

kuusho

Summary

The timelock is set to 90 days in the constant InheritanceManager::TIMELOCK and should be used to update InheritanceManager::setDeadline() after every new contract interaction by the owner.

However this function isn't called after Three different functions

Vulnerability Details

According to the documentation:

"If the Owner does not use his wallet for more than 90 days in this case, his children listed as beneficiaries can call InheritanceManager::inherit() which will enable additional functionality within this contract."

However InheritanceManager::removeBeneficiary(),InheritanceManager::createEstateNft() and InheritanceManager::contractInteractions() do not call InheritanceManager::setDeadline() after execution.

This breaks the logic of the app and lets the owner lose ownership even when they can be considered active.

Here is the code:

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;
}
​
// @audit: This function, like every function that's called only by the owner
// should call `setDeadline()` after the function is called.
// This is because it's a proof of activity from the owner address
}
​
​
function createEstateNFT(string memory _description, uint256 _value, address _asset) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
assetToPay = _asset;
​
// @audit-info: This function, like every function that's called only by the owner
// should call `setDeadline()` after the function is called.
// This is because it's a proof of activity from the owner address
}
​
​
function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
​
// @audit-info: This function, like every function that's called only by the owner
// should call `setDeadline()` after the function is called.
// This is because it's a proof of activity from the owner address
}

Impact

The impact of this particular vulnerability is that InheritanceManager::inherit() may be called by a beneficiary even though the contract owner is still active.

Tools Used

Manual review

Recommendations

call setDeadline() after every function or contract call made bythe owner of the InheritanceManager contract.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Inherit depends on msg.sender so anyone can claim the contract

functions do not reset the deadline

constructor does not initialize deadline

Appeal created

0xtimefliez Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

functions do not reset the deadline

Support

FAQs

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