First shareholder allocation bypasses totalUnclaimedProceeds tracking leading to systematic contract insolvency
The first-shareholder branch in updateShares() allocates proceeds without updating totalUnclaimedProceeds, causing cascading accounting failure where each subsequent first-shareholder allocation ignores prior obligations and leads to insolvency.
Description
The updateShares function contains a critical flaw in the "first shareholder" allocation logic that can lead to systematic contract insolvency. When a shareholder is added as the first shareholder (when totalShares == newShares), the contract allocates proceeds without properly accounting for existing obligations to previous shareholders, causing the contract to promise more funds than it actually holds.
if (totalShares == newShares) {// first shareholder - only allocate unclaimed fundsif (address(this).balance < totalUnclaimedProceeds) revert InsufficientBalance();proceeds[shareholderAddress] = address(this).balance - totalUnclaimedProceeds;// Missing: totalUnclaimedProceeds += proceeds[shareholderAddress];}
The allocation uses totalUnclaimedProceeds to determine "unclaimed funds" but fails to update this variable after the allocation. This creates a cascading accounting failure where each subsequent "first shareholder" allocation ignores previous obligations.
Vulnerable Scenario:
The following steps help reproduce the issue:
- Initial State: Contract receives 100 APE
- Alice Added: Alice becomes first shareholder, gets allocated 100 APE proceeds
- Accounting Error:
totalUnclaimedProceedsremains 0 instead of being updated to 100 APE - Alice Removed: Alice's shares set to 0, but she doesn't withdraw her 100 APE proceeds
- New Funds: Contract receives additional 50 APE (total balance: 150 APE)
- Bob Added: Bob becomes first shareholder
- Broken Allocation: Bob gets
150 - 0 = 150 APEinstead of150 - 100 = 50 APE - Insolvency: Contract owes 250 APE (100 to Alice + 150 to Bob) but only has 150 APE
Impact
This vulnerability creates a scenario where the contract becomes unable to fulfill all withdrawal obligations. The contract allocates more proceeds than its actual balance can support, creating a cascading effect where totalUnclaimedProceeds becomes increasingly inaccurate with each iteration.
This leads to a bank run scenario where early withdrawers receive their funds, but later withdrawers face failed transactions due to insufficient contract balance, resulting in permanent fund loss for some shareholders.
Recommendation
Fix the accounting by properly tracking first shareholder allocations:
if (totalShares == newShares) {// first shareholder - only allocate unclaimed fundsif (address(this).balance < totalUnclaimedProceeds) revert InsufficientBalance();uint256 allocation = address(this).balance - totalUnclaimedProceeds;proceeds[shareholderAddress] = allocation;totalUnclaimedProceeds += allocation; // Critical: Track the allocation}
This ensures the contract maintains accurate accounting of its obligations and prevents systematic insolvency.
Resolution
Golden Grid: Confirmed.
Zealynx: Fixed.

