Skip to content

Latest commit

 

History

History
40 lines (24 loc) · 2.89 KB

063.md

File metadata and controls

40 lines (24 loc) · 2.89 KB

Broad Umber Eagle

Medium

Data parallels structs

Summary

The GovernanceStaker.sol contract presents a security issue related to parallel data structures. The mappings depositorTotalStaked and depositorTotalEarningPower store redundant information, increasing the risk of inconsistencies and incorrect reward calculations, which could lead to potential exploits.

Vulnerability Details

The GovernanceStaker.sol contract uses the mappings depositorTotalStakedand depositorTotalEarningPower to track the total staked balance and the total earning power for each depositor, respectively. These parallel data structures reflect similar information but serve distinct purposes. Their updates occur across several functions, such as _stake, _stakeMore, _alterDelegatee, _withdraw, and _claimReward, increasing the complexity of the logic and the likelihood of synchronization errors.

Reward distribution relies on earning power, which in turn depends on synchronization with the total staked balance, resulting in incorrect reward calculations. Discrepancies between the data structures could be manipulated to gain undue advantages or disrupt the system.

Impact

Failing to maintain consistency between the parallel data structures may result in unequal reward distribution, compromising the system’s integrity and user trust. In extreme cases, an attacker could exploit this vulnerability to drain funds from the contract.

Code Snippet

GovernanceStaker.sol

/// @notice Tracks the total staked by a depositor across all unique deposits.
mapping (address depositor => uint256 amount) public depositorTotalStaked;

/// @notice Tracks the total earning power by a depositor across all unique deposits.
mapping (address depositor => uint256 earningPower) public depositorTotalEarningPower;

Recommendation

To mitigate this issue, it is recommended to eliminate redundancy and centralize update logic.

  • Remove parallel mappings: Eliminate depositorTotalStaked and depositorTotalEarningPower. Instead, store the staked balance and earning power directly within the Deposit structure, simplifying the logic and reducing the risk of errors.
  • Create a unified update function: Implement a single function responsible for updating the staked balance and earning power within the Deposit structure. This function should be called by all methods that modify the deposit state.
  • Implement rigorous testing: Develop comprehensive tests to validate data consistency and reward calculation correctness across various scenarios.

By simplifying the data structure and centralizing update logic, the protocol becomes more robust, secure, and less prone to errors and exploits. Rigorous testing ensures code quality and increases confidence in the system’s correctness.