Summary
A fixed amount of ether is deposited into the depositContract, which is the deposit amount for whiteListed validators (32 eth), and the deposit amount for the non whitelisted validators (16 eth) is not taken into consideration.
Vulnerability Details
When the deposit controller calls the #DepositEther method on EthStakingStrategy, it supplies _nwlTotalValidatorCount and _wlTotalValidatorCount.
function depositEther(
uint256 _nwlTotalValidatorCount,
uint256 _wlTotalValidatorCount,
uint256[] calldata _wlOperatorIds,
uint256[] calldata _wlValidatorCounts
) external {
uint256 totalDepositAmount = (DEPOSIT_AMOUNT *
_wlTotalValidatorCount +
(DEPOSIT_AMOUNT / 2) *
_nwlTotalValidatorCount);
}
Each validator represents either 32 eth or 16 eth deposit dependent on if it is whitelisted or not. as indicated by the code above. nonetheless when the deposit is made to the deposit contract, a fixed amount of 32 eth is deposited.
for (uint256 i = 0; i < _nwlTotalValidatorCount; i++) {
bytes memory pubkey = BytesLib.slice(nwlPubkeys, i * PUBKEY_LENGTH, PUBKEY_LENGTH);
bytes memory signature = BytesLib.slice(
nwlSignatures,
i * SIGNATURE_LENGTH,
SIGNATURE_LENGTH
);
_deposit(pubkey, signature);
}
for (uint256 i = 0; i < _wlTotalValidatorCount; i++) {
bytes memory pubkey = BytesLib.slice(wlPubkeys, i * PUBKEY_LENGTH, PUBKEY_LENGTH);
bytes memory signature = BytesLib.slice(
wlSignatures,
i * SIGNATURE_LENGTH,
SIGNATURE_LENGTH
);
_deposit(pubkey, signature);
}
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/ethStaking/EthStakingStrategy.sol#L218
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/ethStaking/EthStakingStrategy.sol#L228
Each loop of nwl and wl calls the _deposit function which uses a fixed amount of 32 eth, represening WL validators. As seen in depositValue below.
function _deposit(bytes memory _pubkey, bytes memory _signature) internal {
require(withdrawalCredentials != 0, "Empty withdrawal credentials");
uint256 depositValue = DEPOSIT_AMOUNT;
uint256 depositAmount = depositValue / DEPOSIT_AMOUNT_UNIT;
bytes32 pubkeyRoot = sha256(abi.encodePacked(_pubkey, bytes16(0)));
bytes32 signatureRoot = sha256(
abi.encodePacked(
sha256(BytesLib.slice(_signature, 0, 64)),
sha256(
abi.encodePacked(
BytesLib.slice(_signature, 64, SIGNATURE_LENGTH - 64),
bytes32(0)
)
)
)
);
bytes32 depositDataRoot = sha256(
abi.encodePacked(
sha256(abi.encodePacked(pubkeyRoot, withdrawalCredentials)),
sha256(
abi.encodePacked(
_toLittleEndian64(uint64(depositAmount)),
bytes24(0),
signatureRoot
)
)
)
);
uint256 targetBalance = address(this).balance - depositValue;
depositContract.deposit{value: depositValue}(
_pubkey,
abi.encodePacked(withdrawalCredentials),
_signature,
depositDataRoot
);
require(address(this).balance == targetBalance, "Deposit failed");
}
https://github.com/Cyfrin/2024-09-stakelink/blob/f5824f9ad67058b24a2c08494e51ddd7efdbb90b/contracts/ethStaking/EthStakingStrategy.sol#L407
Impact
This will lead to a loss of funds and possible dos of the strategy contract, as much more funds is deposited than intended...
Tools Used
Manual review
Recommendations
_deposit method should accept a boolean value as parameter that indicates if it is a wl or nwl deposit and handle it accordningly.
