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

0 stars 0 forks source link

When a vault is slashed while waiting for withdrawal, vault might not have enough token to withdraw, lead to token stuck #321

Closed c4-bot-4 closed 2 months ago

c4-bot-4 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-07-karak/blob/main/src/Vault.sol#L125-#L149 https://github.com/code-423n4/2024-07-karak/blob/main/src/Vault.sol#L157-#L188

Vulnerability details

Vulnerability details

In vault, to start withdrawal, user need to call startRedeem() function:

function startRedeem(uint256 shares, address beneficiary) external whenFunctionNotPaused(Constants.PAUSE_VAULT_START_REDEEM) nonReentrant returns (bytes32 withdrawalKey)
{
    if (shares == 0) revert ZeroShares();
    if (beneficiary == address(0)) revert ZeroAddress();

    (VaultLib.State storage state, VaultLib.Config storage config) = _storage();
    address staker = msg.sender;

    uint256 assets = convertToAssets(shares);  // <---

    withdrawalKey = WithdrawLib.calculateWithdrawKey(staker, state.stakerToWithdrawNonce[staker]++);

    state.withdrawalMap[withdrawalKey].staker = staker;
    state.withdrawalMap[withdrawalKey].start = uint96(block.timestamp);
    state.withdrawalMap[withdrawalKey].shares = shares;
    state.withdrawalMap[withdrawalKey].beneficiary = beneficiary;

    this.transferFrom(msg.sender, address(this), shares);  // <--- share token transfered to protocol

    emit StartedRedeem(staker, config.operator, shares, withdrawalKey, assets);
}

And wait until withdrawal finish, then call finishRedeem() function:

function finishRedeem(bytes32 withdrawalKey) external nonReentrant whenFunctionNotPaused(Constants.PAUSE_VAULT_FINISH_REDEEM)
{
    (VaultLib.State storage state, VaultLib.Config storage config) = _storage();

    WithdrawLib.QueuedWithdrawal memory startedWithdrawal = state.validateQueuedWithdrawal(withdrawalKey);

    uint256 shares = startedWithdrawal.shares;
    if (shares > maxRedeem(address(this))) revert RedeemMoreThanMax();
    uint256 redeemableAssets = convertToAssets(shares);

    delete state.withdrawalMap[withdrawalKey];

    _withdraw({
        by: address(this),
        to: startedWithdrawal.beneficiary,
        owner: address(this),
        assets: redeemableAssets,
        shares: shares
    });

    emit FinishedRedeem(
        startedWithdrawal.staker,
        startedWithdrawal.beneficiary,
        config.operator,
        startedWithdrawal.shares,
        redeemableAssets,
        withdrawalKey
    );
}

Token will be transfered to startedWithdrawal.beneficiary address. The problem is when vault is slashed, token will be transfered outside of vault:

function slashAssets(uint256 totalAssetsToSlash, address slashingHandler) //@audit race condition neu bi slash
    external
    onlyCore
    returns (uint256 transferAmount)
{
    transferAmount = Math.min(totalAssets(), totalAssetsToSlash);

    // Approve to the handler and then call the handler which will draw the funds
    SafeTransferLib.safeApproveWithRetry(asset(), slashingHandler, transferAmount);
    ISlashingHandler(slashingHandler).handleSlashing(IERC20(asset()), transferAmount);  // <---

    emit Slashed(transferAmount);
}

handleSlashing() function:

function handleSlashing(IERC20 token, uint256 amount) external {
    if (amount == 0) revert ZeroAmount();
    if (!_config().supportedAssets[token]) revert UnsupportedAsset();

    SafeTransferLib.safeTransferFrom(address(token), msg.sender, address(this), amount);
    // Below is where custom logic for each asset lives
    SafeTransferLib.safeTransfer(address(token), address(0), amount);  // <-- token transfered to address(0)
}

When slash is happened between start redeem and finish redeem (which is possible, because MIN_WITHDRAWAL_DELAY = SLASHING_WINDOW + SLASHING_VETO_WINDOW), token can be not enough for user to call finishRedeem() function. Along with share is already transfered to vault, and receive token amount is calculated when start redeem, user's token will be stucked in the vault

Impact

User's token can be stucked in the vault because of slashing.

Tools Used

Manual review.

Recommended Mitigation Steps

Allow user cancel withdrawal period, and transfer share back for user.

Assessed type

Context

c4-bot-7 commented 2 months ago

Withdrawn by grearlake