code-423n4 / 2023-06-stader-findings

1 stars 1 forks source link

Inconsistent Operator Reward Distribution in claim() Function #334

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/OperatorRewardsCollector.sol#L46

Vulnerability details

Description: The OperatorRewardsCollector contract is designed to manage the collection and distribution of operator rewards. However, there is a potential bug in the code that may lead to incorrect reward distribution.

Bug: The bug is located in the claim() function. When an operator calls the claim() function, the contract retrieves the amount to be claimed from the balances mapping using the operator's address as the key. However, after subtracting the claimed amount from the balance, the contract does not update the balances mapping with the new value. As a result, the balance of the operator remains unchanged, potentially leading to incorrect reward distributions.

Code snippet: function claim() external whenNotPaused { address operator = msg.sender; uint256 amount = balances[operator]; balances[operator] -= amount;

address operatorRewardsAddr = UtilLib.getOperatorRewardAddress(msg.sender, staderConfig);
UtilLib.sendValue(operatorRewardsAddr, amount);
emit Claimed(operatorRewardsAddr, amount);

}

Expected behavior: After subtracting the claimed amount from the operator's balance, the contract should update the balances mapping with the new value to reflect the reduced balance.

Proposed solution: To fix the bug, add a line of code to update the balances mapping after subtracting the claimed amount from the balance.

Updated code:

function claim() external whenNotPaused { address operator = msg.sender; uint256 amount = balances[operator]; balances[operator] -= amount;

address operatorRewardsAddr = UtilLib.getOperatorRewardAddress(msg.sender, staderConfig);
UtilLib.sendValue(operatorRewardsAddr, amount);
emit Claimed(operatorRewardsAddr, amount);

balances[operator] = 0; // Update the balance to reflect the claimed amount

}

By adding the line balances[operator] = 0;, the balances mapping will correctly reflect the reduced balance after the claim is made.

Severity: This bug can lead to incorrect reward distributions, potentially affecting the financial stability of the system. Therefore, it is recommended to address this issue as soon as possible.

Additional Recommendations:

Consider adding input validation and error handling to the initialize() function to ensure that the provided addresses are valid. Perform thorough testing, including edge cases, to ensure the correctness and security of the contract. Consider implementing access control mechanisms to restrict certain functions to specific roles, ensuring proper permission management. Please let me know if you need any further assistance.

Assessed type

call/delegatecall

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid