From 915e9bc0b0b26bf6050541cb438435b520832d69 Mon Sep 17 00:00:00 2001 From: Houmgaor Date: Tue, 10 Mar 2026 11:19:22 +0100 Subject: [PATCH] fix(gacha): prevent panics from misconfigured gacha entries (#175) getRandomEntries panics with rand.Intn(0) when box gacha has more rolls than available entries. Fix by clamping rolls, guarding empty entries and zero-weight pools, and fixing an out-of-bounds slice in the receive handler overflow path. --- server/channelserver/handlers_gacha.go | 8 ++++-- server/channelserver/handlers_gacha_test.go | 32 +++++++++++++++++++++ server/channelserver/svc_gacha.go | 26 +++++++++++++++-- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/server/channelserver/handlers_gacha.go b/server/channelserver/handlers_gacha.go index 24f2448ac..fe8c61678 100644 --- a/server/channelserver/handlers_gacha.go +++ b/server/channelserver/handlers_gacha.go @@ -88,8 +88,10 @@ func handleMsgMhfReceiveGachaItem(s *Session, p mhfpacket.MHFPacket) { data = []byte{0x00} } - // I think there are still some edge cases where rewards can be nulled via overflow - if data[0] > 36 || len(data) > 181 { + // Handle overflow: the client can only display 36 items (36 * 5 + 1 count byte = 181 bytes). + // If there are more, send the first 36 and keep the rest for next time. + isOverflow := len(data) > 181 && data[0] > 36 + if isOverflow { resp := byteframe.NewByteFrame() resp.WriteUint8(36) resp.WriteBytes(data[1:181]) @@ -99,7 +101,7 @@ func handleMsgMhfReceiveGachaItem(s *Session, p mhfpacket.MHFPacket) { } if !pkt.Freeze { - if data[0] > 36 || len(data) > 181 { + if isOverflow { update := byteframe.NewByteFrame() update.WriteUint8(uint8(len(data[181:]) / 5)) update.WriteBytes(data[181:]) diff --git a/server/channelserver/handlers_gacha_test.go b/server/channelserver/handlers_gacha_test.go index 1c6c60814..c05f09c31 100644 --- a/server/channelserver/handlers_gacha_test.go +++ b/server/channelserver/handlers_gacha_test.go @@ -597,6 +597,38 @@ func TestGetRandomEntries_Box(t *testing.T) { } } +func TestGetRandomEntries_BoxMoreRollsThanEntries(t *testing.T) { + entries := []GachaEntry{ + {ID: 1, Weight: 50}, + } + result, err := getRandomEntries(entries, 5, true) + if err != nil { + t.Fatal(err) + } + // Should clamp to available entries instead of panicking + if len(result) != 1 { + t.Errorf("Expected 1 entry (clamped), got %d", len(result)) + } +} + +func TestGetRandomEntries_EmptyEntries(t *testing.T) { + _, err := getRandomEntries(nil, 1, false) + if err == nil { + t.Fatal("Expected error for empty entries, got nil") + } +} + +func TestGetRandomEntries_ZeroWeight(t *testing.T) { + entries := []GachaEntry{ + {ID: 1, Weight: 0}, + {ID: 2, Weight: 0}, + } + _, err := getRandomEntries(entries, 1, false) + if err == nil { + t.Fatal("Expected error for zero total weight, got nil") + } +} + func TestHandleMsgMhfPlayStepupGacha_RewardPoolError(t *testing.T) { server := createMockServer() charRepo := newMockCharacterRepo() diff --git a/server/channelserver/svc_gacha.go b/server/channelserver/svc_gacha.go index 1085a1791..7908af26b 100644 --- a/server/channelserver/svc_gacha.go +++ b/server/channelserver/svc_gacha.go @@ -103,11 +103,17 @@ func (svc *GachaService) spendGachaCoin(userID uint32, quantity uint16) { // resolveRewards selects random entries and resolves them into rewards. func (svc *GachaService) resolveRewards(entries []GachaEntry, rolls int, isBox bool) []GachaReward { - rewardEntries, _ := getRandomEntries(entries, rolls, isBox) + rewardEntries, err := getRandomEntries(entries, rolls, isBox) + if err != nil { + svc.logger.Warn("Failed to select gacha entries", zap.Error(err)) + return nil + } var rewards []GachaReward for i := range rewardEntries { entryItems, err := svc.gachaRepo.GetItemsForEntry(rewardEntries[i].ID) if err != nil { + svc.logger.Warn("Gacha entry has no items", + zap.Uint32("entryID", rewardEntries[i].ID), zap.Error(err)) continue } for _, item := range entryItems { @@ -228,11 +234,17 @@ func (svc *GachaService) PlayBoxGacha(userID, charID, gachaID uint32, rollType u if err != nil { return nil, err } - rewardEntries, _ := getRandomEntries(entries, rolls, true) + rewardEntries, err := getRandomEntries(entries, rolls, true) + if err != nil { + svc.logger.Warn("Failed to select box gacha entries", zap.Error(err)) + return &GachaPlayResult{}, nil + } var rewards []GachaReward for i := range rewardEntries { entryItems, err := svc.gachaRepo.GetItemsForEntry(rewardEntries[i].ID) if err != nil { + svc.logger.Warn("Box gacha entry has no items", + zap.Uint32("entryID", rewardEntries[i].ID), zap.Error(err)) continue } if err := svc.gachaRepo.InsertBoxEntry(gachaID, rewardEntries[i].ID, charID); err != nil { @@ -299,11 +311,21 @@ func (svc *GachaService) ResetBox(gachaID, charID uint32) error { // chosen with weighted probability (with replacement). In box mode, entries are // chosen uniformly without replacement. func getRandomEntries(entries []GachaEntry, rolls int, isBox bool) ([]GachaEntry, error) { + if len(entries) == 0 { + return nil, errors.New("no gacha entries available") + } + // Box mode draws without replacement, so clamp rolls to available entries. + if isBox && rolls > len(entries) { + rolls = len(entries) + } var chosen []GachaEntry var totalWeight float64 for i := range entries { totalWeight += entries[i].Weight } + if !isBox && totalWeight <= 0 { + return nil, errors.New("gacha entries have zero total weight") + } for rolls != len(chosen) { if !isBox { result := rand.Float64() * totalWeight