Description

The handler for FundCommunityPool allows users to send coins to the community pool:

func (k msgServer) FundCommunityPool(ctx context.Context, msg *types.MsgFundCommunityPool) (*types.MsgFundCommunityPoolResponse, error) {
    defer telemetry.MeasureSince(time.Now(), "distribution", "msg", "fund-community-pool")

    depositor, err := k.authKeeper.AddressCodec().StringToBytes(msg.Depositor)
    if err != nil {
        return nil, err
    }
    if err := k.Keeper.FundCommunityPool(ctx, msg.Amount, depositor); err != nil {
        return nil, err
    }

    return &types.MsgFundCommunityPoolResponse{}, nil
}

The issue is that the msg.Amount is not validated to ensure that every value is positive and that there are no duplicate coins. The same issue exists in the CommunityPoolSpend handler.

Impact

If a negative value is used, the call to MoveBankKeeper.SendCoin will end up failing when trying to convert the amount to a uint64. If duplicate coins are used, the call to SendCoin will succeed, but luckily the call to sdk.NewDecCoinsFromCoins when saving the new community pool will end up failing. As code further down the line may expect the coins to already be validated, it would be better to ensure that the coins are validated before being used.

Recommendations

The msg.Amount for both FundCommunityPool and CommunityPoolSpend should be validated, and an appropriate error message should be returned.

Remediation