sherlock-audit / 2024-06-allora-judging

0 stars 0 forks source link

imsrybr0 - SetDelegateStakePlacement error is not handled in RewardDelegateStake #30

Open sherlock-admin2 opened 4 months ago

sherlock-admin2 commented 4 months ago

imsrybr0

High

SetDelegateStakePlacement error is not handled in RewardDelegateStake

Summary

SetDelegateStakePlacement error is not handled in RewardDelegateStake.

Vulnerability Detail

If SetDelegateStakePlacement fails in RewardDelegateStake, the RewardDebt will not be saved and it would allow a delegate staker to claim the same rewards more than once.

Impact

Rewards being distributed more than once can lead to insolvency (.i.e : others delegate stakers not being able to claim their rewards) and the drainage of the AlloraPendingReward module account.

Code Snippet

RewardDelegateStake

func (ms msgServer) RewardDelegateStake(ctx context.Context, msg *types.MsgRewardDelegateStake) (*types.MsgRewardDelegateStakeResponse, error) {
    // Check the target reputer exists and is registered
    isRegistered, err := ms.k.IsReputerRegisteredInTopic(ctx, msg.TopicId, msg.Reputer)
    if err != nil {
        return nil, err
    }
    if !isRegistered {
        return nil, types.ErrAddressIsNotRegisteredInThisTopic
    }

    delegateInfo, err := ms.k.GetDelegateStakePlacement(ctx, msg.TopicId, msg.Sender, msg.Reputer)
    if err != nil {
        return nil, err
    }
    share, err := ms.k.GetDelegateRewardPerShare(ctx, msg.TopicId, msg.Reputer)
    if err != nil {
        return nil, err
    }
    pendingReward, err := delegateInfo.Amount.Mul(share)
    if err != nil {
        return nil, err
    }
    pendingReward, err = pendingReward.Sub(delegateInfo.RewardDebt) // <===== Audit
    if err != nil {
        return nil, err
    }
    if pendingReward.Gt(alloraMath.NewDecFromInt64(0)) {
        coins := sdk.NewCoins(sdk.NewCoin(params.DefaultBondDenom, pendingReward.SdkIntTrim()))
        err = ms.k.SendCoinsFromModuleToAccount(ctx, types.AlloraPendingRewardForDelegatorAccountName, msg.Sender, coins)
        if err != nil {
            return nil, err
        }
        delegateInfo.RewardDebt, err = delegateInfo.Amount.Mul(share)
        if err != nil {
            return nil, err
        }
        ms.k.SetDelegateStakePlacement(ctx, msg.TopicId, msg.Sender, msg.Reputer, delegateInfo) // <====== Audit
    }
    return &types.MsgRewardDelegateStakeResponse{}, nil
}

Tool used

Manual Review

Recommendation

Handle theSetDelegateStakePlacement error.

sherlock-admin2 commented 4 months ago

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

Error SetDelegateStakePlacement is not handled correctly in RewardDelegateStake

sherlock-admin2 commented 4 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/allora-network/allora-chain/pull/463