mirror of
https://github.com/Mezeporta/Erupe.git
synced 2026-03-22 07:32:32 +01:00
fix: array bound crashes on clans.
This commit is contained in:
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
- Config file handling and validation
|
||||
- Fixes 3 critical race condition in handlers_stage.go.
|
||||
- Fix an issue causing a crash on clans with 0 members.
|
||||
|
||||
### Security
|
||||
|
||||
|
||||
@@ -3,4 +3,4 @@
|
||||
Before submitting a new version:
|
||||
|
||||
- Document your changes in [CHANGELOG.md](CHANGELOG.md).
|
||||
- Run tests: `go test -v ./...`
|
||||
- Run tests: `go test -v ./...` and check for race conditions: `go test -v -race ./...`
|
||||
|
||||
@@ -86,7 +86,18 @@ func encodeServerInfo(config *_config.Config, s *Server, local bool) []byte {
|
||||
}
|
||||
}
|
||||
bf.WriteUint32(uint32(channelserver.TimeAdjusted().Unix()))
|
||||
bf.WriteUint32(uint32(s.erupeConfig.GameplayOptions.ClanMemberLimits[len(s.erupeConfig.GameplayOptions.ClanMemberLimits)-1][1]))
|
||||
|
||||
// ClanMemberLimits requires at least 1 element with 2 columns to avoid index out of range panics
|
||||
// Use default value (60) if array is empty or last row is too small
|
||||
var maxClanMembers uint8 = 60
|
||||
if len(s.erupeConfig.GameplayOptions.ClanMemberLimits) > 0 {
|
||||
lastRow := s.erupeConfig.GameplayOptions.ClanMemberLimits[len(s.erupeConfig.GameplayOptions.ClanMemberLimits)-1]
|
||||
if len(lastRow) > 1 {
|
||||
maxClanMembers = lastRow[1]
|
||||
}
|
||||
}
|
||||
bf.WriteUint32(uint32(maxClanMembers))
|
||||
|
||||
return bf.Data()
|
||||
}
|
||||
|
||||
|
||||
171
server/entranceserver/make_resp_test.go
Normal file
171
server/entranceserver/make_resp_test.go
Normal file
@@ -0,0 +1,171 @@
|
||||
package entranceserver
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"go.uber.org/zap"
|
||||
|
||||
_config "erupe-ce/config"
|
||||
)
|
||||
|
||||
// TestEncodeServerInfo_EmptyClanMemberLimits verifies the crash is FIXED when ClanMemberLimits is empty
|
||||
// Previously panicked: runtime error: index out of range [-1]
|
||||
// From erupe.log.1:659922
|
||||
// After fix: Should handle empty array gracefully with default value (60)
|
||||
func TestEncodeServerInfo_EmptyClanMemberLimits(t *testing.T) {
|
||||
config := &_config.Config{
|
||||
RealClientMode: _config.Z1,
|
||||
Host: "127.0.0.1",
|
||||
Entrance: _config.Entrance{
|
||||
Enabled: true,
|
||||
Port: 53310,
|
||||
Entries: []_config.EntranceServerInfo{
|
||||
{
|
||||
Name: "TestServer",
|
||||
Description: "Test",
|
||||
IP: "127.0.0.1",
|
||||
Type: 0,
|
||||
Recommended: 0,
|
||||
AllowedClientFlags: 0xFFFFFFFF,
|
||||
Channels: []_config.EntranceChannelInfo{
|
||||
{
|
||||
Port: 54001,
|
||||
MaxPlayers: 100,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
GameplayOptions: _config.GameplayOptions{
|
||||
ClanMemberLimits: [][]uint8{}, // Empty array - should now use default (60) instead of panicking
|
||||
},
|
||||
}
|
||||
|
||||
server := &Server{
|
||||
logger: zap.NewNop(),
|
||||
erupeConfig: config,
|
||||
}
|
||||
|
||||
// Set up defer to catch ANY panic - we should NOT get array bounds panic anymore
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
// If panic occurs, it should NOT be from array access
|
||||
panicStr := fmt.Sprintf("%v", r)
|
||||
if strings.Contains(panicStr, "index out of range") {
|
||||
t.Errorf("Array bounds panic NOT fixed! Still getting: %v", r)
|
||||
} else {
|
||||
// Other panic is acceptable (network, DB, etc) - we only care about array bounds
|
||||
t.Logf("Non-array-bounds panic (acceptable): %v", r)
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
// This should NOT panic on array bounds anymore - should use default value 60
|
||||
result := encodeServerInfo(config, server, true)
|
||||
if len(result) > 0 {
|
||||
t.Log("✅ encodeServerInfo handled empty ClanMemberLimits without array bounds panic")
|
||||
}
|
||||
}
|
||||
|
||||
// TestClanMemberLimitsBoundsChecking verifies bounds checking logic for ClanMemberLimits
|
||||
// Tests the specific logic that was fixed without needing full database setup
|
||||
func TestClanMemberLimitsBoundsChecking(t *testing.T) {
|
||||
// Test the bounds checking logic directly
|
||||
testCases := []struct {
|
||||
name string
|
||||
clanMemberLimits [][]uint8
|
||||
expectedValue uint8
|
||||
expectDefault bool
|
||||
}{
|
||||
{"empty array", [][]uint8{}, 60, true},
|
||||
{"single row with 2 columns", [][]uint8{{1, 50}}, 50, false},
|
||||
{"single row with 1 column", [][]uint8{{1}}, 60, true},
|
||||
{"multiple rows, last has 2 columns", [][]uint8{{1, 10}, {2, 20}, {3, 60}}, 60, false},
|
||||
{"multiple rows, last has 1 column", [][]uint8{{1, 10}, {2, 20}, {3}}, 60, true},
|
||||
{"multiple rows with valid data", [][]uint8{{1, 10}, {2, 20}, {3, 30}, {4, 40}, {5, 50}}, 50, false},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
// Replicate the bounds checking logic from the fix
|
||||
var maxClanMembers uint8 = 60
|
||||
if len(tc.clanMemberLimits) > 0 {
|
||||
lastRow := tc.clanMemberLimits[len(tc.clanMemberLimits)-1]
|
||||
if len(lastRow) > 1 {
|
||||
maxClanMembers = lastRow[1]
|
||||
}
|
||||
}
|
||||
|
||||
// Verify correct behavior
|
||||
if maxClanMembers != tc.expectedValue {
|
||||
t.Errorf("Expected value %d, got %d", tc.expectedValue, maxClanMembers)
|
||||
}
|
||||
|
||||
if tc.expectDefault && maxClanMembers != 60 {
|
||||
t.Errorf("Expected default value 60, got %d", maxClanMembers)
|
||||
}
|
||||
|
||||
t.Logf("✅ %s: Safe bounds access, value = %d", tc.name, maxClanMembers)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// TestEncodeServerInfo_MissingSecondColumnClanMemberLimits tests accessing [last][1] when [last] is too small
|
||||
// Previously panicked: runtime error: index out of range [1]
|
||||
// After fix: Should handle missing column gracefully with default value (60)
|
||||
func TestEncodeServerInfo_MissingSecondColumnClanMemberLimits(t *testing.T) {
|
||||
config := &_config.Config{
|
||||
RealClientMode: _config.Z1,
|
||||
Host: "127.0.0.1",
|
||||
Entrance: _config.Entrance{
|
||||
Enabled: true,
|
||||
Port: 53310,
|
||||
Entries: []_config.EntranceServerInfo{
|
||||
{
|
||||
Name: "TestServer",
|
||||
Description: "Test",
|
||||
IP: "127.0.0.1",
|
||||
Type: 0,
|
||||
Recommended: 0,
|
||||
AllowedClientFlags: 0xFFFFFFFF,
|
||||
Channels: []_config.EntranceChannelInfo{
|
||||
{
|
||||
Port: 54001,
|
||||
MaxPlayers: 100,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
GameplayOptions: _config.GameplayOptions{
|
||||
ClanMemberLimits: [][]uint8{
|
||||
{1}, // Only 1 element, code used to panic accessing [1]
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
server := &Server{
|
||||
logger: zap.NewNop(),
|
||||
erupeConfig: config,
|
||||
}
|
||||
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
panicStr := fmt.Sprintf("%v", r)
|
||||
if strings.Contains(panicStr, "index out of range") {
|
||||
t.Errorf("Array bounds panic NOT fixed! Still getting: %v", r)
|
||||
} else {
|
||||
t.Logf("Non-array-bounds panic (acceptable): %v", r)
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
// This should NOT panic on array bounds anymore - should use default value 60
|
||||
result := encodeServerInfo(config, server, true)
|
||||
if len(result) > 0 {
|
||||
t.Log("✅ encodeServerInfo handled missing ClanMemberLimits column without array bounds panic")
|
||||
}
|
||||
}
|
||||
@@ -338,10 +338,17 @@ func (s *Session) makeSignResponse(uid uint32) []byte {
|
||||
bf.WriteBytes(stringsupport.PaddedString(psnUser, 20, true))
|
||||
}
|
||||
|
||||
bf.WriteUint16(s.server.erupeConfig.DebugOptions.CapLink.Values[0])
|
||||
if s.server.erupeConfig.DebugOptions.CapLink.Values[0] == 51728 {
|
||||
bf.WriteUint16(s.server.erupeConfig.DebugOptions.CapLink.Values[1])
|
||||
if s.server.erupeConfig.DebugOptions.CapLink.Values[1] == 20000 || s.server.erupeConfig.DebugOptions.CapLink.Values[1] == 20002 {
|
||||
// CapLink.Values requires at least 5 elements to avoid index out of range panics
|
||||
// Provide safe defaults if array is too small
|
||||
capLinkValues := s.server.erupeConfig.DebugOptions.CapLink.Values
|
||||
if len(capLinkValues) < 5 {
|
||||
capLinkValues = []uint16{0, 0, 0, 0, 0}
|
||||
}
|
||||
|
||||
bf.WriteUint16(capLinkValues[0])
|
||||
if capLinkValues[0] == 51728 {
|
||||
bf.WriteUint16(capLinkValues[1])
|
||||
if capLinkValues[1] == 20000 || capLinkValues[1] == 20002 {
|
||||
ps.Uint16(bf, s.server.erupeConfig.DebugOptions.CapLink.Key, false)
|
||||
}
|
||||
}
|
||||
@@ -356,10 +363,10 @@ func (s *Session) makeSignResponse(uid uint32) []byte {
|
||||
bf.WriteUint32(caStruct[i].Unk1)
|
||||
ps.Uint8(bf, caStruct[i].Unk2, false)
|
||||
}
|
||||
bf.WriteUint16(s.server.erupeConfig.DebugOptions.CapLink.Values[2])
|
||||
bf.WriteUint16(s.server.erupeConfig.DebugOptions.CapLink.Values[3])
|
||||
bf.WriteUint16(s.server.erupeConfig.DebugOptions.CapLink.Values[4])
|
||||
if s.server.erupeConfig.DebugOptions.CapLink.Values[2] == 51729 && s.server.erupeConfig.DebugOptions.CapLink.Values[3] == 1 && s.server.erupeConfig.DebugOptions.CapLink.Values[4] == 20000 {
|
||||
bf.WriteUint16(capLinkValues[2])
|
||||
bf.WriteUint16(capLinkValues[3])
|
||||
bf.WriteUint16(capLinkValues[4])
|
||||
if capLinkValues[2] == 51729 && capLinkValues[3] == 1 && capLinkValues[4] == 20000 {
|
||||
ps.Uint16(bf, fmt.Sprintf(`%s:%d`, s.server.erupeConfig.DebugOptions.CapLink.Host, s.server.erupeConfig.DebugOptions.CapLink.Port), false)
|
||||
}
|
||||
|
||||
|
||||
213
server/signserver/dsgn_resp_test.go
Normal file
213
server/signserver/dsgn_resp_test.go
Normal file
@@ -0,0 +1,213 @@
|
||||
package signserver
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"go.uber.org/zap"
|
||||
|
||||
_config "erupe-ce/config"
|
||||
)
|
||||
|
||||
// TestMakeSignResponse_EmptyCapLinkValues verifies the crash is FIXED when CapLink.Values is empty
|
||||
// Previously panicked: runtime error: index out of range [0] with length 0
|
||||
// From erupe.log.1:659796 and 659853
|
||||
// After fix: Should handle empty array gracefully with defaults
|
||||
func TestMakeSignResponse_EmptyCapLinkValues(t *testing.T) {
|
||||
config := &_config.Config{
|
||||
DebugOptions: _config.DebugOptions{
|
||||
CapLink: _config.CapLinkOptions{
|
||||
Values: []uint16{}, // Empty array - should now use defaults instead of panicking
|
||||
Key: "test",
|
||||
Host: "localhost",
|
||||
Port: 8080,
|
||||
},
|
||||
},
|
||||
GameplayOptions: _config.GameplayOptions{
|
||||
MezFesSoloTickets: 100,
|
||||
MezFesGroupTickets: 100,
|
||||
ClanMemberLimits: [][]uint8{
|
||||
{1, 10},
|
||||
{2, 20},
|
||||
{3, 30},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
session := &Session{
|
||||
logger: zap.NewNop(),
|
||||
server: &Server{
|
||||
erupeConfig: config,
|
||||
logger: zap.NewNop(),
|
||||
},
|
||||
client: PC100,
|
||||
}
|
||||
|
||||
// Set up defer to catch ANY panic - we should NOT get array bounds panic anymore
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
// If panic occurs, it should NOT be from array access
|
||||
panicStr := fmt.Sprintf("%v", r)
|
||||
if strings.Contains(panicStr, "index out of range") {
|
||||
t.Errorf("Array bounds panic NOT fixed! Still getting: %v", r)
|
||||
} else {
|
||||
// Other panic is acceptable (DB, etc) - we only care about array bounds
|
||||
t.Logf("Non-array-bounds panic (acceptable): %v", r)
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
// This should NOT panic on array bounds anymore
|
||||
result := session.makeSignResponse(0)
|
||||
if result != nil && len(result) > 0 {
|
||||
t.Log("✅ makeSignResponse handled empty CapLink.Values without array bounds panic")
|
||||
}
|
||||
}
|
||||
|
||||
// TestMakeSignResponse_InsufficientCapLinkValues verifies the crash is FIXED when CapLink.Values is too small
|
||||
// Previously panicked: runtime error: index out of range [1]
|
||||
// After fix: Should handle small array gracefully with defaults
|
||||
func TestMakeSignResponse_InsufficientCapLinkValues(t *testing.T) {
|
||||
config := &_config.Config{
|
||||
DebugOptions: _config.DebugOptions{
|
||||
CapLink: _config.CapLinkOptions{
|
||||
Values: []uint16{51728}, // Only 1 element, code used to panic accessing [1]
|
||||
Key: "test",
|
||||
Host: "localhost",
|
||||
Port: 8080,
|
||||
},
|
||||
},
|
||||
GameplayOptions: _config.GameplayOptions{
|
||||
MezFesSoloTickets: 100,
|
||||
MezFesGroupTickets: 100,
|
||||
ClanMemberLimits: [][]uint8{
|
||||
{1, 10},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
session := &Session{
|
||||
logger: zap.NewNop(),
|
||||
server: &Server{
|
||||
erupeConfig: config,
|
||||
logger: zap.NewNop(),
|
||||
},
|
||||
client: PC100,
|
||||
}
|
||||
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
panicStr := fmt.Sprintf("%v", r)
|
||||
if strings.Contains(panicStr, "index out of range") {
|
||||
t.Errorf("Array bounds panic NOT fixed! Still getting: %v", r)
|
||||
} else {
|
||||
t.Logf("Non-array-bounds panic (acceptable): %v", r)
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
// This should NOT panic on array bounds anymore
|
||||
result := session.makeSignResponse(0)
|
||||
if result != nil && len(result) > 0 {
|
||||
t.Log("✅ makeSignResponse handled insufficient CapLink.Values without array bounds panic")
|
||||
}
|
||||
}
|
||||
|
||||
// TestMakeSignResponse_MissingCapLinkValues234 verifies the crash is FIXED when CapLink.Values doesn't have 5 elements
|
||||
// Previously panicked: runtime error: index out of range [2/3/4]
|
||||
// After fix: Should handle small array gracefully with defaults
|
||||
func TestMakeSignResponse_MissingCapLinkValues234(t *testing.T) {
|
||||
config := &_config.Config{
|
||||
DebugOptions: _config.DebugOptions{
|
||||
CapLink: _config.CapLinkOptions{
|
||||
Values: []uint16{100, 200}, // Only 2 elements, code used to panic accessing [2][3][4]
|
||||
Key: "test",
|
||||
Host: "localhost",
|
||||
Port: 8080,
|
||||
},
|
||||
},
|
||||
GameplayOptions: _config.GameplayOptions{
|
||||
MezFesSoloTickets: 100,
|
||||
MezFesGroupTickets: 100,
|
||||
ClanMemberLimits: [][]uint8{
|
||||
{1, 10},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
session := &Session{
|
||||
logger: zap.NewNop(),
|
||||
server: &Server{
|
||||
erupeConfig: config,
|
||||
logger: zap.NewNop(),
|
||||
},
|
||||
client: PC100,
|
||||
}
|
||||
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
panicStr := fmt.Sprintf("%v", r)
|
||||
if strings.Contains(panicStr, "index out of range") {
|
||||
t.Errorf("Array bounds panic NOT fixed! Still getting: %v", r)
|
||||
} else {
|
||||
t.Logf("Non-array-bounds panic (acceptable): %v", r)
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
// This should NOT panic on array bounds anymore
|
||||
result := session.makeSignResponse(0)
|
||||
if result != nil && len(result) > 0 {
|
||||
t.Log("✅ makeSignResponse handled missing CapLink.Values[2/3/4] without array bounds panic")
|
||||
}
|
||||
}
|
||||
|
||||
// TestCapLinkValuesBoundsChecking verifies bounds checking logic for CapLink.Values
|
||||
// Tests the specific logic that was fixed without needing full database setup
|
||||
func TestCapLinkValuesBoundsChecking(t *testing.T) {
|
||||
// Test the bounds checking logic directly
|
||||
testCases := []struct {
|
||||
name string
|
||||
values []uint16
|
||||
expectDefault bool
|
||||
}{
|
||||
{"empty array", []uint16{}, true},
|
||||
{"1 element", []uint16{100}, true},
|
||||
{"2 elements", []uint16{100, 200}, true},
|
||||
{"3 elements", []uint16{100, 200, 300}, true},
|
||||
{"4 elements", []uint16{100, 200, 300, 400}, true},
|
||||
{"5 elements (valid)", []uint16{100, 200, 300, 400, 500}, false},
|
||||
{"6 elements (valid)", []uint16{100, 200, 300, 400, 500, 600}, false},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
// Replicate the bounds checking logic from the fix
|
||||
capLinkValues := tc.values
|
||||
if len(capLinkValues) < 5 {
|
||||
capLinkValues = []uint16{0, 0, 0, 0, 0}
|
||||
}
|
||||
|
||||
// Verify all 5 indices are now safe to access
|
||||
_ = capLinkValues[0]
|
||||
_ = capLinkValues[1]
|
||||
_ = capLinkValues[2]
|
||||
_ = capLinkValues[3]
|
||||
_ = capLinkValues[4]
|
||||
|
||||
// Verify correct behavior
|
||||
if tc.expectDefault {
|
||||
if capLinkValues[0] != 0 || capLinkValues[1] != 0 {
|
||||
t.Errorf("Expected default values, got %v", capLinkValues)
|
||||
}
|
||||
} else {
|
||||
if capLinkValues[0] == 0 && tc.values[0] != 0 {
|
||||
t.Errorf("Expected original values, got defaults")
|
||||
}
|
||||
}
|
||||
|
||||
t.Logf("✅ %s: All 5 indices accessible without panic", tc.name)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user