Summary
If only one user is added it will be treated as a backup wallet but not as inheritance, the problem arises when one mistakenly adds the backup wallet address more than once so the backup wallet cases is treated as inheritance case and ownership is not transfered to the backup wallet address.
Vulnerability Details
This is correct case of backup wallet where one address is added as backup wallet and after the timeout the ownership is transferd to the backup wallet address.
function test_inheritOnlyOneBeneficiary() public {
console.log("Owner address: ", im.getOwner());
vm.startPrank(owner);
im.addBeneficiery(user1);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 10e10);
vm.warp(1 + 90 days);
vm.startPrank(user1);
im.inherit();
vm.stopPrank();
console.log("Owner address: ", im.getOwner());
assertEq(user1, im.getOwner());
}
This is the outcome of above test.
[PASS] test_inheritOnlyOneBeneficiary() (gas: 95423)
Logs:
Owner address: 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
Owner address: 0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF
But if the backup address is added twice, it should still be considered as backup address because it is same address but since there is no check on adding duplicated users the ownership is not transferred to backup wallet and this is not the intended functionality as seen from the test below.
function test_inheritOnlyOneBeneficiaryAddedTwice() public {
vm.startPrank(owner);
console.log("Owner address: ", im.getOwner());
im.addBeneficiery(user1);
im.addBeneficiery(user1);
vm.stopPrank();
vm.warp(1);
vm.deal(address(im), 10e10);
vm.warp(1 + 90 days);
vm.startPrank(user1);
im.inherit();
vm.stopPrank();
console.log("Owner address: ", im.getOwner());
assertEq(user1, im.getOwner());
}
Outcome of the failed test.
[FAIL: assertion failed: 0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF != 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266] test_inheritOnlyOneBeneficiaryAddedTwice() (gas: 138197)
Logs:
Owner address: 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
Owner address: 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
Traces:
[138197] InheritanceManagerTest::test_inheritOnlyOneBeneficiaryAddedTwice()
├─ [0] VM::startPrank(owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266])
│ └─ ← [Return]
├─ [2419] InheritanceManager::getOwner() [staticcall]
│ └─ ← [Return] owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]
├─ [0] console::log("Owner address: ", owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]) [staticcall]
│ └─ ← [Stop]
├─ [67020] InheritanceManager::addBeneficiery(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Stop]
├─ [23120] InheritanceManager::addBeneficiery(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(1)
│ └─ ← [Return]
├─ [0] VM::deal(InheritanceManager: [0x88F59F8826af5e695B13cA934d6c7999875A9EeA], 100000000000 [1e11])
│ └─ ← [Return]
├─ [0] VM::warp(7776001 [7.776e6])
│ └─ ← [Return]
├─ [0] VM::startPrank(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF])
│ └─ ← [Return]
├─ [22686] InheritanceManager::inherit()
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [419] InheritanceManager::getOwner() [staticcall]
│ └─ ← [Return] owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]
├─ [0] console::log("Owner address: ", owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]) [staticcall]
│ └─ ← [Stop]
├─ [419] InheritanceManager::getOwner() [staticcall]
│ └─ ← [Return] owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]
├─ [0] VM::assertEq(user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], owner: [0x7c8999dC9a822c1f0Df42023113EDB4FDd543266]) [staticcall]
│ └─ ← [Revert] assertion failed: 0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF != 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
└─ ← [Revert] assertion failed: 0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF != 0x7c8999dC9a822c1f0Df42023113EDB4FDd543266
Impact
This error deviates from the intended functionality and the ownership of the vault should be transferred to the backup address but its not.
Tools Used
foundry test.
Recommendations
addBeneficiery function should check if an address is already added to the list and should not add it again.
