Unauthorized Stake Remove Cancellation of Reputors or Workers by any user

Summary

The Allora AppChain's CancelRemoveStake & RemoveDelegateStake function, allowing unauthorized users to cancel the stake removal process of any reputer or worker or their delegators.

Vulnerability Detail

The CancelRemoveStake function in the Allora AppChain is designed to allow reputers to cancel their stake removal requests during the delay window. However, the function only verifies if the message sender is associated with the stake removal process without confirming if the sender is indeed the valid signer of the message or has the authority to cancel the stake removal. https://github.com/allora-network/allora-chain/blob/3a97afe7af027c96749fac7c4327ae85359a61c8/x/emissions/keeper/msgserver/msg_server_stake.go#L115-L132

// cancel a request to remove your stake, during the delay window
func (ms msgServer) CancelRemoveStake(ctx context.Context, msg *types.MsgCancelRemoveStake) (*types.MsgCancelRemoveStakeResponse, error) {
    sdkCtx := sdk.UnwrapSDKContext(ctx)
    removal, found, err := ms.k.GetStakeRemovalForReputerAndTopicId(sdkCtx, msg.Sender, msg.TopicId)
    // if the specific error is that we somehow got into a buggy invariant state
    // where more than one removal request exists in the queue
    // still allow people to cancel withdrawing their stake (fail open rather than closed)
    if err != nil && !errors.Is(err, types.ErrInvariantFailure) {
        return nil, errorsmod.Wrap(err, "error while searching previous stake removal")
    }
    if !found {
        return nil, types.ErrStakeRemovalNotFound
    }
    err = ms.k.DeleteStakeRemoval(ctx, removal.BlockRemovalCompleted, removal.TopicId, removal.Reputer)
    if err != nil {
        return nil, errorsmod.Wrap(err, "failed to delete previous stake removal")
    }
    return &types.MsgCancelRemoveStakeResponse{}, nil
}

https://github.com/allora-network/allora-chain/blob/3a97afe7af027c96749fac7c4327ae85359a61c8/x/emissions/keeper/msgserver/msg_server_stake.go#L243-L268

func (ms msgServer) CancelRemoveDelegateStake(ctx context.Context, msg *types.MsgCancelRemoveDelegateStake) (*types.MsgCancelRemoveDelegateStakeResponse, error) {
    sdkCtx := sdk.UnwrapSDKContext(ctx)
    removal, found, err := ms.k.GetDelegateStakeRemovalForDelegatorReputerAndTopicId(
        sdkCtx, msg.Sender, msg.Reputer, msg.TopicId,
    )
    // if the specific error is that we somehow got into a buggy invariant state
    // where more than one removal request exists in the queue
    // still allow people to cancel withdrawing their stake (fail open rather than closed)
    if err != nil && !errors.Is(err, types.ErrInvariantFailure) {
        return nil, errorsmod.Wrap(err, "error while searching previous delegate stake removal")
    }
    if !found {
        return nil, types.ErrStakeRemovalNotFound
    }
    err = ms.k.DeleteDelegateStakeRemoval(
        ctx,
        removal.BlockRemovalCompleted,
        removal.TopicId,
        removal.Reputer,
        removal.Delegator,
    )
    if err != nil {
        return nil, errorsmod.Wrap(err, "failed to delete previous delegate stake removal")
    }
    return &types.MsgCancelRemoveDelegateStakeResponse{}, nil
}

Impact

This vulnerability can have severe impacts on the Allora AppChain, including: Malicious actors can cancel the stake removal process of any reputer or worker, effectively forcing reputers and delegators to keep their stakes locked indefinitely. This can prevent users from withdrawing their funds, leading to a denial of service condition where users cannot access their own assets.

Tool used

Manual Review

Recommendation

It is recommended to implement proper verification of the message sender's identity and authority before allowing the stake removal cancellation.

Duplicate of #130

sherlock-audit / 2024-06-allora-judging