Project

Summary

The state management in the MembershipERC1155 contract's profit distribution system contains a critical vulnerability where state changes in saveProfit occur before the transfer is completed. If the transfer fails, the contract's state remains altered, leading to permanent loss of user profit claims. This creates a severe economic impact where users could lose their rightful profit shares due to failed transactions, with no mechanism to recover these lost claims. The issue fundamentally breaks the profit distribution system's accounting and could result in significant financial losses for token holders.

The saveProfit function prematurely updates critical state variables lastProfit and savedProfit before ensuring the successful completion of the profit transfer. This violates the principle of atomic state updates where all operations in a transaction should either complete successfully or revert entirely. The current implementation updates the profit tracking state before the external token transfer, creating a window where state and reality become misaligned if the transfer fails. This misalignment becomes permanent as the state changes persist even when the transfer reverts, effectively erasing the user's claim to their accumulated profits.

Vulnerable Code Snippet

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L144
https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/tokens/MembershipERC1155.sol#L182

Scenario

An account has accumulated 100 tokens worth of unclaimed profits. They initiate a claimProfit transaction. The saveProfit function executes, updating lastProfit[account] to the current totalProfit and setting savedProfit[account] to include their unclaimed profits. However, the token transfer fails due to insufficient contract balance. The transaction reverts, but the state updates from saveProfit have already occurred. The account's profit claim is now permanently lost because lastProfit has been updated to the current totalProfit, effectively resetting their claim window, and their savedProfit value remains unchanged despite the failed transfer. Any subsequent attempts to claim will show zero available profits, as the contract believes the profits were already claimed due to the updated lastProfit value.

Recommended Fix

Additionally, consider implementing a profit claim queue system where claims are tracked separately from the main profit accounting system, providing an additional layer of safety and auditability for profit distributions.