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

Missing Revert in Beneficiary Validation

teoslaf

Summary

The onlyBeneficiaryWithIsInherited modifier lacks a revert condition, causing it to silently pass even when the caller is not a beneficiary, completely breaking access control.

Vulnerability Details

modifier onlyBeneficiaryWithIsInherited() {
uint256 i = 0;
while (i < beneficiaries.length + 1) {
if (msg.sender == beneficiaries[i] && isInherited) {
break; // Breaks loop but continues execution!
}
i++;
}
_; // CRITICAL: No revert if not found!
}

Critical issues:

  1. Missing Validation

    • No revert condition after loop

    • Loop break doesn't prevent execution

    • Silently continues even if not found

    • Complete access control bypass

  2. Logic Flow Issues

    • Break statement only exits loop

    • No validation of loop result

    • No check if match was found

    • Always allows execution

  3. Affected Functions

function buyOutEstateNFT(uint256 _nftID)
external
onlyBeneficiaryWithIsInherited
{ ... }
​
function appointTrustee(address _trustee)
external
onlyBeneficiaryWithIsInherited
{ ... }

Impact

CRITICAL - The vulnerability enables:

  1. Complete Access Control Bypass

    • Any address can access functions

    • No beneficiary validation

    • No inheritance status check

    • Full system compromise

  2. System Exposure

    • NFT buyout system unprotected

    • Trustee appointment unprotected

    • Asset theft possible

    • System control loss

Proof of Concept

contract AccessTest {
InheritanceManager target;
function exploit() external {
// Can call "protected" functions from any address
target.buyOutEstateNFT(1); // Works!
target.appointTrustee(address(this)); // Works!
// Modifier silently passes because:
// 1. Loop runs but doesn't find us
// 2. No revert condition
// 3. Execution continues
}
}

Recommendations

  1. Add Proper Validation:

modifier onlyBeneficiaryWithIsInherited() {
require(isInherited, "Not inherited");
bool found = false;
for(uint i = 0; i < beneficiaries.length; i++) {
if(msg.sender == beneficiaries[i]) {
found = true;
break;
}
}
require(found, "Not a beneficiary");
_;
}

  1. Use Safe Data Structures:

contract InheritanceManager {
mapping(address => bool) public isBeneficiary;
modifier onlyBeneficiaryWithInherited() {
require(isInherited, "Not inherited");
require(isBeneficiary[msg.sender], "Not beneficiary");
_;
}
}

  1. Improve Error Handling:

    • Clear error messages

    • Proper state validation

    • Event emission

    • Access logging

Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!