From 87aa7cf643746689d4ff07d50084e71a685aacdb Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 26 Sep 2024 08:38:34 +0000 Subject: [PATCH] fix: slither ci checks --- contracts/OperatorRewardsCollector.sol | 17 +++++++++++------ contracts/PermissionedPool.sol | 1 + contracts/PermissionlessNodeRegistry.sol | 5 +++++ contracts/PermissionlessPool.sol | 1 + contracts/SocializingPool.sol | 1 + contracts/StaderInsuranceFund.sol | 1 + contracts/StaderStakePoolsManager.sol | 1 + contracts/VaultProxy.sol | 2 +- 8 files changed, 22 insertions(+), 7 deletions(-) diff --git a/contracts/OperatorRewardsCollector.sol b/contracts/OperatorRewardsCollector.sol index 7a318eb2..decdaf8e 100644 --- a/contracts/OperatorRewardsCollector.sol +++ b/contracts/OperatorRewardsCollector.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.16; import { Math } from "@openzeppelin/contracts/utils/math/Math.sol"; import { AccessControlUpgradeable } from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; +import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; import { UtilLib } from "./library/UtilLib.sol"; @@ -17,7 +18,7 @@ import { ISDCollateral } from "./interfaces/SDCollateral/ISDCollateral.sol"; import { IWETH } from "./interfaces/IWETH.sol"; import { IStaderOracle } from "../contracts/interfaces/IStaderOracle.sol"; -contract OperatorRewardsCollector is IOperatorRewardsCollector, AccessControlUpgradeable { +contract OperatorRewardsCollector is IOperatorRewardsCollector, AccessControlUpgradeable, ReentrancyGuard { IStaderConfig public staderConfig; mapping(address => uint256) public balances; @@ -144,19 +145,23 @@ contract OperatorRewardsCollector is IOperatorRewardsCollector, AccessControlUpg balances[operator], operatorLiquidation.totalAmountInEth - operatorLiquidation.totalFeeInEth ); + + uint256 protocolFee = Math.min(operatorLiquidation.totalFeeInEth, balances[operator]); + + // Effects + balances[operator] -= wETHDeposit; + balances[operator] -= protocolFee; weth.deposit{ value: wETHDeposit }(); if (weth.transferFrom(address(this), operatorLiquidation.liquidator, wETHDeposit) == false) revert WethTransferFailed(); - balances[operator] -= wETHDeposit; - uint256 protocolFee = Math.min(operatorLiquidation.totalFeeInEth, balances[operator]); UtilLib.sendValue(staderConfig.getStaderTreasury(), protocolFee); - balances[operator] -= protocolFee; sdUtilityPool.completeLiquidation(operator); } else { // Transfer WETH to liquidator and ETH to treasury + balances[operator] -= operatorLiquidation.totalAmountInEth; weth.deposit{ value: operatorLiquidation.totalAmountInEth - operatorLiquidation.totalFeeInEth }(); if ( weth.transferFrom( @@ -168,7 +173,6 @@ contract OperatorRewardsCollector is IOperatorRewardsCollector, AccessControlUpg UtilLib.sendValue(staderConfig.getStaderTreasury(), operatorLiquidation.totalFeeInEth); sdUtilityPool.completeLiquidation(operator); - balances[operator] -= operatorLiquidation.totalAmountInEth; } } } @@ -180,10 +184,11 @@ contract OperatorRewardsCollector is IOperatorRewardsCollector, AccessControlUpg * @param operator The address of the operator claiming the amount. * @param amount The amount to be claimed. */ - function _claim(address operator, uint256 amount) internal { + function _claim(address operator, uint256 amount) internal nonReentrant{ uint256 maxWithdrawableInEth = withdrawableInEth(operator); if (amount > maxWithdrawableInEth || amount > balances[operator]) revert InsufficientBalance(); + //slither-disable-next-line reentrancy-eth balances[operator] -= amount; // If there's an amount to send, transfer it to the operator's rewards address diff --git a/contracts/PermissionedPool.sol b/contracts/PermissionedPool.sol index 51e13681..0a625fa3 100644 --- a/contracts/PermissionedPool.sol +++ b/contracts/PermissionedPool.sol @@ -272,6 +272,7 @@ contract PermissionedPool is IStaderPoolBase, AccessControlUpgradeable, Reentran } function increasePreDepositValidatorCount(uint256 _count) internal { + //slither-disable-next-line reentrancy-eth preDepositValidatorCount += _count; } diff --git a/contracts/PermissionlessNodeRegistry.sol b/contracts/PermissionlessNodeRegistry.sol index e1605eea..87fa156d 100644 --- a/contracts/PermissionlessNodeRegistry.sol +++ b/contracts/PermissionlessNodeRegistry.sol @@ -228,6 +228,7 @@ contract PermissionlessNodeRegistry is } if (frontRunValidatorsLength > 0) { + //slither-disable-next-line arbitrary-send-eth IStaderInsuranceFund(staderConfig.getStaderInsuranceFund()).depositFund{ value: frontRunValidatorsLength * FRONT_RUN_PENALTY }(); @@ -440,6 +441,7 @@ contract PermissionlessNodeRegistry is */ function transferCollateralToPool(uint256 _amount) external override nonReentrant { UtilLib.onlyStaderContract(msg.sender, staderConfig, staderConfig.PERMISSIONLESS_POOL()); + //slither-disable-next-line arbitrary-send-eth IPermissionlessPool(staderConfig.getPermissionlessPool()).receiveRemainingCollateralETH{ value: _amount }(); emit TransferredCollateralToPool(_amount); } @@ -658,6 +660,7 @@ contract PermissionlessNodeRegistry is // handle front run validator by changing their status, deactivating operator and imposing penalty function handleFrontRun(uint256 _validatorId) internal { + //slither-disable-next-line reentrancy-eth validatorRegistry[_validatorId].status = ValidatorStatus.FRONT_RUN; uint256 operatorId = validatorRegistry[_validatorId].operatorId; operatorStructById[operatorId].active = false; @@ -666,9 +669,11 @@ contract PermissionlessNodeRegistry is // handle validator with invalid signature for 1ETH deposit //send back remaining ETH to operator address function handleInvalidSignature(uint256 _validatorId) internal { + //slither-disable-next-line reentrancy-eth validatorRegistry[_validatorId].status = ValidatorStatus.INVALID_SIGNATURE; uint256 operatorId = validatorRegistry[_validatorId].operatorId; address operatorAddress = operatorStructById[operatorId].operatorAddress; + //slither-disable-next-line arbitrary-send-eth IOperatorRewardsCollector(staderConfig.getOperatorRewardsCollector()).depositFor{ value: (COLLATERAL_ETH - staderConfig.getPreDepositSize()) }(operatorAddress); diff --git a/contracts/PermissionlessPool.sol b/contracts/PermissionlessPool.sol index 6a211e43..0d2e6bda 100644 --- a/contracts/PermissionlessPool.sol +++ b/contracts/PermissionlessPool.sol @@ -257,6 +257,7 @@ contract PermissionlessPool is IStaderPoolBase, AccessControlUpgradeable, Reentr withdrawCredential, _DEPOSIT_SIZE ); + //slither-disable-next-line arbitrary-send-eth IDepositContract(_ethDepositContract).deposit{ value: _DEPOSIT_SIZE }( pubkey, withdrawCredential, diff --git a/contracts/SocializingPool.sol b/contracts/SocializingPool.sol index 2c919eb6..742ccacf 100644 --- a/contracts/SocializingPool.sol +++ b/contracts/SocializingPool.sol @@ -83,6 +83,7 @@ contract SocializingPool is lastReportedRewardsData = _rewardsData; rewardsDataMap[_rewardsData.index] = _rewardsData; + //slither-disable-next-line arbitrary-send-eth IStaderStakePoolManager(staderConfig.getStakePoolManager()).receiveExecutionLayerRewards{ value: _rewardsData.userETHRewards }(); diff --git a/contracts/StaderInsuranceFund.sol b/contracts/StaderInsuranceFund.sol index a1fc5bf6..77debf1e 100644 --- a/contracts/StaderInsuranceFund.sol +++ b/contracts/StaderInsuranceFund.sol @@ -57,6 +57,7 @@ contract StaderInsuranceFund is IStaderInsuranceFund, AccessControlUpgradeable, if (address(this).balance < _amount) { revert InSufficientBalance(); } + //slither-disable-next-line arbitrary-send-eth IPermissionedPool(staderConfig.getPermissionedPool()).receiveInsuranceFund{ value: _amount }(); } diff --git a/contracts/StaderStakePoolsManager.sol b/contracts/StaderStakePoolsManager.sol index efb47894..2f6111bc 100644 --- a/contracts/StaderStakePoolsManager.sol +++ b/contracts/StaderStakePoolsManager.sol @@ -256,6 +256,7 @@ contract StaderStakePoolsManager is address poolAddress = IPoolUtils(poolUtils).poolAddressById(poolIdArray[i]); uint256 poolDepositSize = ETH_PER_NODE - IPoolUtils(poolUtils).getCollateralETH(poolIdArray[i]); + //slither-disable-next-line reentrancy-eth lastExcessETHDepositBlock = block.number; //slither-disable-next-line arbitrary-send-eth IStaderPoolBase(poolAddress).stakeUserETHToBeaconChain{ value: validatorToDeposit * poolDepositSize }(); diff --git a/contracts/VaultProxy.sol b/contracts/VaultProxy.sol index 1f2936e6..c04e78de 100644 --- a/contracts/VaultProxy.sol +++ b/contracts/VaultProxy.sol @@ -41,7 +41,7 @@ contract VaultProxy is IVaultProxy { address vaultImplementation = isValidatorWithdrawalVault ? staderConfig.getValidatorWithdrawalVaultImplementation() : staderConfig.getNodeELRewardVaultImplementation(); - // solhint-disable-next-line avoid-low-level-calls + //slither-disable-next-line controlled-delegatecall (bool success, bytes memory data) = vaultImplementation.delegatecall(_input); if (!success) { revert(string(data));