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

Denial of Service (DoS) Risk Due to Long Beneficiary List in onlyBeneficiaryWithIsInherited Modifier

Summary

The InheritanceManager::onlyBeneficiaryWithIsInherited modifier in the InheritanceManager contract iterates over the beneficiaries array using a while loop. If the number of beneficiaries is excessively large, the function execution may consume an excessive amount of gas, potentially exceeding the block gas limit. This could lead to a denial of service (DoS) attack, preventing beneficiaries from executing inheritance-related functions such as appointTrustee and buyOutEstateNFT.


Vulnerability Details

modifier onlyBeneficiaryWithIsInherited() {
uint256 i = 0;
// @audit-issue Unbounded iteration over the `beneficiaries` array leads to high gas costs and potential DoS
@> while (i < beneficiaries.length + 1) {
if (msg.sender == beneficiaries[i] && isInherited) {
break;
}
i++;
}
_;
}

Issues Identified

  1. Unbounded Iteration

    • The while loop iterates through every element in beneficiaries. If the array becomes too large, the gas cost for each function call using this modifier will scale linearly.

    • If beneficiaries.length becomes too large, execution will require more gas than the block gas limit, causing permanent function failure.

  2. Denial of Service (DoS) Risk

    • If the number of beneficiaries is too high, no function protected by this modifier will be executable due to exceeding gas limits.

    • This can result in beneficiaries being unable to withdraw inherited funds or perform essential operations.


Impact

🚨 Critical Consequences

  • Denial of Service (DoS): If the beneficiaries array grows too large, any function using onlyBeneficiaryWithIsInherited will require excessive gas to iterate through the list. This can cause the function execution to exceed the block gas limit, making it impossible for beneficiaries to invoke critical inheritance-related functions.

  • Locked Funds: If protected functions such as appointTrustee and buyOutEstateNFT become uncallable due to gas exhaustion, beneficiaries may be unable to access or distribute inherited assets, effectively locking funds within the contract.

  • Inefficient Execution: Every call to functions using this modifier will incur unnecessary gas costs as the loop iterates through all beneficiaries. This inefficiency could discourage users from interacting with the contract and increase transaction fees unnecessarily.


Proof of Concept (PoC)

PoC Explanation

PoC Explanation

  1. Populate the Beneficiary List

    • The test begins by adding 42,000 beneficiaries to the contract using addBeneficiery.

    • This ensures that any function requiring iteration over the beneficiaries list will have high gas consumption.

    • Each beneficiary is assigned a unique address (address(uint160(i + 1000))).

  2. Simulate the Inheritance Activation

    • The test warps time forward by 90 days, fulfilling the condition to mark the contract as "inherited."

    • A valid beneficiary (address(1000)) calls inherit(), setting isInherited = true.

  3. Trigger a Function with the DoS Risk

    • Another beneficiary (address(40000)) attempts to call appointTrustee, which uses the onlyBeneficiaryWithIsInherited modifier.

    • Since there are 42,000 beneficiaries, the modifier must iterate through the entire array to confirm the sender is in the list.

    • This results in excessive gas consumption, eventually leading to an out-of-gas (OOG) error.

  4. Expected Outcome

    • The transaction should fail due to gas exhaustion when executing appointTrustee().

    • The test uses vm.expectRevert(); to ensure that the transaction fails as expected, confirming that functions with onlyBeneficiaryWithIsInherited become unusable when too many beneficiaries exist.

    • If the transaction unexpectedly succeeds, the test will fail, highlighting a potential gas efficiency issue.

function test_DOS_DueToLargeBeneficiaryList() public {
for (uint256 i = 0; i < 42_000; i++) {
vm.prank(owner);
im.addBeneficiery(address(uint160(i + 1000)));
}
vm.warp(1 + 90 days);
vm.prank(address(1000));
im.inherit();
address trusteeAddr = makeAddr("Trustee");
vm.prank(address(40000));
vm.expectRevert(); //Out of gas error
im.appointTrustee(trusteeAddr);
}

Tools Used

  • Manual Review

  • Foundry Unit Tests


Recommendations

Instead of looping through an array, use a mapping(address => bool) for O(1) lookup time:

mapping(address => bool) public isBeneficiary;
​
modifier onlyBeneficiaryWithIsInherited() {
require(isInherited, "Not inherited yet");
require(isBeneficiary[msg.sender], "Not a beneficiary");
_;
}
​
function addBeneficiery(address _beneficiary) external onlyOwner {
require(!isBeneficiary[_beneficiary], "Already a beneficiary");
isBeneficiary[_beneficiary] = true;
beneficiaries.push(_beneficiary);
}

āœ” Reduces gas costs from O(n) to O(1).
āœ” Eliminates DoS risk from excessive iteration.


Updates

Lead Judging Commences

0xtimefliez Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!