code-423n4 / 2024-07-karak-validation

0 stars 0 forks source link

Missing check for validator slashed status #246

Closed c4-bot-10 closed 2 months ago

c4-bot-10 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/entities/NativeVaultLib.sol#L146-L193 https://github.com/code-423n4/2024-07-karak/blob/f5e52fdcb4c20c4318d532a9f08f7876e9afb321/src/NativeVault.sol#L168-L204

Vulnerability details

The code currently lacks a check for whether a validator has been slashed. This omission could allow a slashed validator to be treated as active or valid, potentially leading to incorrect behavior in the system. Specifically, a slashed validator may be erroneously considered for withdrawals or staking operations, which could undermine the protocol's integrity and security.

Impact

The issue is exacerbated by the fact that the exit epoch is not set immediately upon slashing (here), allowing slashed validators to bypass the existing checks.

Proof of Concept

The following code snippet demonstrates the missing check:

    function validateWithdrawalCredentials(
        Storage storage self,
        address nodeOwner,
        uint64 updateTimestamp,
        bytes32 beaconStateRoot,
        BeaconProofs.ValidatorFieldsProof calldata validatorFieldsProof
    ) internal returns (uint256) {
        bytes32 validatorPubkeyHash = BeaconProofs.getPubkeyHash(validatorFieldsProof.validatorFields);
        NativeVaultLib.ValidatorDetails memory validatorDetails =
            self.ownerToNode[nodeOwner].validatorPubkeyHashToDetails[validatorPubkeyHash];

        if (validatorDetails.status != NativeVaultLib.ValidatorStatus.INACTIVE) revert ValidatorAlreadyActive();
        if (BeaconProofs.getExitEpoch(validatorFieldsProof.validatorFields) != type(uint64).max) {
            revert ValidatorExiting();
        }
        // Construct beacon chain withdraw address with current node's payable address
        if (
            BeaconProofs.getWithdrawalCredentials(validatorFieldsProof.validatorFields)
                != bytes32(abi.encodePacked(bytes1(uint8(1)), bytes11(0), address(self.ownerToNode[nodeOwner].nodeAddress)))
        ) {
            revert WithdrawalCredentialsMismatchWithNode();
        }

        .............
    }

Tools Used

Manual code review

Recommended Mitigation Steps

Implement a check to verify if a validator has been slashed by examining the slashed status in the validator's fields. This can be done by inspecting the appropriate hash tree root values. If the validator has been slashed, appropriate actions should be taken to prevent the validator from being treated as active or eligible for certain operations, such as restaking or withdrawals.

if (BeaconProofs.getSlashedStatus(validatorFieldsProof.validatorFields)) {
    revert ValidatorSlashed();
}

This addition ensures that slashed validators are correctly identified and handled according to protocol rules, thereby maintaining the integrity and security of the system.

Assessed type

Invalid Validation

tpiliposian commented 2 months ago

Could someone please take a look at this?