Description

Accounts that can be modified by cross program invocations (CPI) must be explicitly deserialized again after the CPI is performed. withdraw_stake invokes the token program to transfer tokens from the custody account to the user withdrawal account. After the CPI, the amount from the custody account is passed to validate without refreshing the information read from the account. The amount passed to validate is therefore the amount before the tokens are transferred.

transfer(
    CpiContext::from(&*ctx.accounts).with_signer(&[&[
        AUTHORITY_SEED.as_bytes(),
        ctx.accounts.stake_account_positions.key().as_ref(),
        &[stake_account_metadata.authority_bump],
    ]]),
    amount,
)?;

if utils::risk::validate(
    stake_account_positions,
!!!    stake_account_custody.amount,
    unvested_balance,
    current_epoch,
    config.unlocking_duration,
)

Impact

This mistake makes the call to validate occurring after the transfer effectively useless. Fortunately the issue is not exploitable, as the function performs a call to validate immediately before the token transfer, using a computed amount corresponding to the amount after the transfer.

Recommendations

Currently, the call to validate after transfer does not have any effect; it cannot cause a transaction to revert, since the condition for a revert would have already been caught by the call to validate before the transfer. Therefore removing the call to validate after transfer is a valid remediation.

Alternatively, we recommend calling inserting a call to .reload() immediately after the call to transfer.

Remediation

TBD: Pending client feedback.