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

Multiple functions in `InheritanceManager` do not reset 90 day timelock breaking core invariant

blackgrease

Summary

There are 3 owner functions within the `InheritanceManager.sol` contract that break the core invariant - "EVERY transaction the owner does with this contract must reset the 90 days timer ". This results in an incorrect deadline being set even though the owner is active and funds being inheritable incorrectly.

Vulnerability Details

The Protocol declares that one of the core invariant's is “ EVERY transaction the owner does with this contract must reset the 90 days timer”. This core invariant is broken by the following 3 functions in InheritanceManager.sol

  • contractInteractions(): L120

  • createEstateNFT(): L143

  • removeBeneficiary(): L163

When any of the above functions are called by the owner, they do not reset the 90 day timer. If an owner, only interacts with any of these 3 functions - which are very important - the deadline will never be correctly set and even though they are active, the funds/assets will become inheritable to beneficiaries therefore breaking the contracts functionality.

Impact

The impact of this bug is High because it breaks the core invariant of the Protocol. Furthermore, if the owner does not interact with any functions that correctly reset the timer, then beneficiaries will be able to “inherit” funds while an owner is still active further breaking the contracts logic and purpose.

PoC

I have outlined the code for one of the functions that have been correctly implemented - addBeneficiery - and compared it to the 3 functions that break the core invariant due to missing the _setDeadline() function call.

A Correct implementation of addBeneficiery owner function implemented correctly with the _setDeadline()

​
function addBeneficiery(address _beneficiary) external onlyOwner {
beneficiaries.push(_beneficiary);
_setDeadline(); //timer is correctly re-set
}

Incorrect implementation of contractInteractions owner function

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;
}//missing _setDeadline()
}

Incorrect implementation of createEstateNFT owner function

​
function createEstateNFT(string memory _description, uint256 _value, address _asset) external onlyOwner {
uint256 nftID = nft.createEstate(_description);
nftValue[nftID] = _value;
assetToPay = _asset;
} //missing _setDeadline()
​

Incorrect implementation of removeBeneficiary owner function

​
function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
} //missing _setDeadline()
​

Tools Used

This vulnerability was found using:

  • Manual Code Review

Recommendations

The recommend mitigation to fix this issue is to implement the function call to the internal function _setDeadline() within the 3 vulnerable functions

  • createEstateNFT

  • contractInteractions

  • removeBeneficiary

Using removeBeneficiary() as an example

function removeBeneficiary(address _beneficiary) external onlyOwner {
uint256 indexToRemove = _getBeneficiaryIndex(_beneficiary);
delete beneficiaries[indexToRemove];
+ _setDeadline()
}
​
Updates

Lead Judging Commences

0xtimefliez Lead Judge 3 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.