Lines of code

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

Vulnerability details

Impact

The claim() function has a reentrancy vulnerability . In the function, the UtilLib.sendValue() function is called before emitting the Claimed event. This violates the "check-effect-interaction" model, which is a best practice for secure smart contract development.

contracts/OperatorRewardsCollector.sol:
  45  
  46:     function claim() external whenNotPaused {
  47:         address operator = msg.sender;
  48:         uint256 amount = balances[operator];
  49:         balances[operator] -= amount;
  50: 
  51:         address operatorRewardsAddr = UtilLib.getOperatorRewardAddress(msg.sender, staderConfig);
  52:         UtilLib.sendValue(operatorRewardsAddr, amount); // @audit send ether
  53:         emit Claimed(operatorRewardsAddr, amount);
  54:     }

Tools Used

Manuel Code Review

Recommended Mitigation Steps

By updating the contract state and emitting the event before the interaction with operatorRewardsAddr, you ensure that no state changes or unexpected interactions occur after the Ether transfer. This mitigates the reentrancy vulnerability and aligns with the recommended "check-effect-interaction" model.


contracts/OperatorRewardsCollector.sol:
  45  
  46:     function claim() external whenNotPaused {
  47:         address operator = msg.sender;
  48:         uint256 amount = balances[operator];
  49:         balances[operator] -= amount;
  50: 
  51:         address operatorRewardsAddr = UtilLib.getOperatorRewardAddress(msg.sender, staderConfig);
-  52:        UtilLib.sendValue(operatorRewardsAddr, amount);
  53:         emit Claimed(operatorRewardsAddr, amount);
+             UtilLib.sendValue(operatorRewardsAddr, amount);
         }

Assessed type

Reentrancy

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