From ac59188488c2dca51331795bbc154bb95bb98279 Mon Sep 17 00:00:00 2001
From: Houmgaor <contact@houmgaor.com>
Date: Fri, 20 Feb 2026 20:00:54 +0100
Subject: [PATCH] refactor(byteframe): replace read-overflow panic with sticky
 error

ByteFrame previously panicked on out-of-bounds reads, which crashed
the server when parsing malformed client packets. Now sets a sticky
error (checked via Err()) and returns zero values, matching the
encoding/binary scanner pattern. The session recv loop checks Err()
after parsing to reject malformed packets gracefully.

Also replaces remaining panic("Not implemented") stubs in network
packet Build/Parse methods with proper error returns.
---
 common/byteframe/byteframe.go                 | 58 ++++++++++++++++++-
 common/byteframe/byteframe_test.go            | 32 +++++++---
 network/binpacket/msg_bin_mail_notify.go      |  4 +-
 network/mhfpacket/msg_sys_get_stage_binary.go |  4 +-
 network/mhfpacket/msg_sys_move_stage.go       |  6 +-
 network/mhfpacket/msg_sys_set_stage_binary.go |  4 +-
 .../mhfpacket/msg_sys_wait_stage_binary.go    |  4 +-
 server/channelserver/sys_session.go           |  8 +++
 8 files changed, 105 insertions(+), 15 deletions(-)

diff --git a/common/byteframe/byteframe.go b/common/byteframe/byteframe.go
index 94478441a..1c2a11214 100644
--- a/common/byteframe/byteframe.go
+++ b/common/byteframe/byteframe.go
@@ -9,16 +9,21 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"fmt"
 	"io"
 	"math"
 )
 
+// ErrReadOverflow is returned when a read exceeds the buffer bounds.
+var ErrReadOverflow = errors.New("byteframe: read beyond buffer bounds")
+
 // ByteFrame is a struct for reading and writing raw byte data.
 type ByteFrame struct {
 	index     uint
 	usedSize  uint
 	buf       []byte
 	byteOrder binary.ByteOrder
+	err       error // sticky error set on read overflow
 }
 
 // NewByteFrame creates a new ByteFrame with valid default values.
@@ -92,7 +97,14 @@ func (b *ByteFrame) rprologue(size uint) {
 }
 
 func (b *ByteFrame) rerr() {
-	panic("Error while reading!")
+	if b.err == nil {
+		b.err = fmt.Errorf("%w: at index %d, usedSize %d", ErrReadOverflow, b.index, b.usedSize)
+	}
+}
+
+// Err returns the first read error encountered, if any.
+func (b *ByteFrame) Err() error {
+	return b.err
 }
 
 // Seek (implements the io.Seeker interface)
@@ -247,8 +259,12 @@ func (b *ByteFrame) WriteNullTerminatedBytes(x []byte) {
 
 // ReadUint8 reads a uint8 at the current index.
 func (b *ByteFrame) ReadUint8() (x uint8) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(1) {
 		b.rerr()
+		return 0
 	}
 	x = uint8(b.buf[b.index])
 	b.rprologue(1)
@@ -265,8 +281,12 @@ func (b *ByteFrame) ReadBool() (x bool) {
 
 // ReadUint16 reads a uint16 at the current index.
 func (b *ByteFrame) ReadUint16() (x uint16) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(2) {
 		b.rerr()
+		return 0
 	}
 	x = b.byteOrder.Uint16(b.buf[b.index:])
 	b.rprologue(2)
@@ -275,8 +295,12 @@ func (b *ByteFrame) ReadUint16() (x uint16) {
 
 // ReadUint32 reads a uint32 at the current index.
 func (b *ByteFrame) ReadUint32() (x uint32) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(4) {
 		b.rerr()
+		return 0
 	}
 	x = b.byteOrder.Uint32(b.buf[b.index:])
 	b.rprologue(4)
@@ -285,8 +309,12 @@ func (b *ByteFrame) ReadUint32() (x uint32) {
 
 // ReadUint64 reads a uint64 at the current index.
 func (b *ByteFrame) ReadUint64() (x uint64) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(8) {
 		b.rerr()
+		return 0
 	}
 	x = b.byteOrder.Uint64(b.buf[b.index:])
 	b.rprologue(8)
@@ -295,8 +323,12 @@ func (b *ByteFrame) ReadUint64() (x uint64) {
 
 // ReadInt8 reads a int8 at the current index.
 func (b *ByteFrame) ReadInt8() (x int8) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(1) {
 		b.rerr()
+		return 0
 	}
 	x = int8(b.buf[b.index])
 	b.rprologue(1)
@@ -305,8 +337,12 @@ func (b *ByteFrame) ReadInt8() (x int8) {
 
 // ReadInt16 reads a int16 at the current index.
 func (b *ByteFrame) ReadInt16() (x int16) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(2) {
 		b.rerr()
+		return 0
 	}
 	x = int16(b.byteOrder.Uint16(b.buf[b.index:]))
 	b.rprologue(2)
@@ -315,8 +351,12 @@ func (b *ByteFrame) ReadInt16() (x int16) {
 
 // ReadInt32 reads a int32 at the current index.
 func (b *ByteFrame) ReadInt32() (x int32) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(4) {
 		b.rerr()
+		return 0
 	}
 	x = int32(b.byteOrder.Uint32(b.buf[b.index:]))
 	b.rprologue(4)
@@ -325,8 +365,12 @@ func (b *ByteFrame) ReadInt32() (x int32) {
 
 // ReadInt64 reads a int64 at the current index.
 func (b *ByteFrame) ReadInt64() (x int64) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(8) {
 		b.rerr()
+		return 0
 	}
 	x = int64(b.byteOrder.Uint64(b.buf[b.index:]))
 	b.rprologue(8)
@@ -335,8 +379,12 @@ func (b *ByteFrame) ReadInt64() (x int64) {
 
 // ReadFloat32 reads a float32 at the current index.
 func (b *ByteFrame) ReadFloat32() (x float32) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(4) {
 		b.rerr()
+		return 0
 	}
 	x = math.Float32frombits(b.byteOrder.Uint32(b.buf[b.index:]))
 	b.rprologue(4)
@@ -345,8 +393,12 @@ func (b *ByteFrame) ReadFloat32() (x float32) {
 
 // ReadFloat64 reads a float64 at the current index.
 func (b *ByteFrame) ReadFloat64() (x float64) {
+	if b.err != nil {
+		return 0
+	}
 	if !b.rcheck(8) {
 		b.rerr()
+		return 0
 	}
 	x = math.Float64frombits(b.byteOrder.Uint64(b.buf[b.index:]))
 	b.rprologue(8)
@@ -355,8 +407,12 @@ func (b *ByteFrame) ReadFloat64() (x float64) {
 
 // ReadBytes reads `size` many bytes at the current index.
 func (b *ByteFrame) ReadBytes(size uint) (x []byte) {
+	if b.err != nil {
+		return nil
+	}
 	if !b.rcheck(size) {
 		b.rerr()
+		return nil
 	}
 	x = b.buf[b.index : b.index+size]
 	b.rprologue(size)
diff --git a/common/byteframe/byteframe_test.go b/common/byteframe/byteframe_test.go
index 423b204ff..52c1449c1 100644
--- a/common/byteframe/byteframe_test.go
+++ b/common/byteframe/byteframe_test.go
@@ -3,6 +3,7 @@ package byteframe
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"io"
 	"math"
 	"testing"
@@ -430,18 +431,33 @@ func TestByteFrame_BufferGrowth(t *testing.T) {
 	}
 }
 
-func TestByteFrame_ReadPanic(t *testing.T) {
-	defer func() {
-		if r := recover(); r == nil {
-			t.Error("Reading beyond buffer should panic")
-		}
-	}()
-
+func TestByteFrame_ReadOverflowSetsError(t *testing.T) {
 	bf := NewByteFrame()
 	bf.WriteUint8(0x01)
 	_, _ = bf.Seek(0, io.SeekStart)
 	bf.ReadUint8()
-	bf.ReadUint16() // Should panic - trying to read 2 bytes when only 1 was written
+
+	if bf.Err() != nil {
+		t.Fatal("Err() should be nil before overflow")
+	}
+
+	// Should set sticky error - trying to read 2 bytes when only 1 was written
+	got := bf.ReadUint16()
+	if got != 0 {
+		t.Errorf("ReadUint16() after overflow = %d, want 0", got)
+	}
+	if bf.Err() == nil {
+		t.Error("Err() should be non-nil after read overflow")
+	}
+	if !errors.Is(bf.Err(), ErrReadOverflow) {
+		t.Errorf("Err() = %v, want ErrReadOverflow", bf.Err())
+	}
+
+	// Subsequent reads should also return zero without changing the error
+	got32 := bf.ReadUint32()
+	if got32 != 0 {
+		t.Errorf("ReadUint32() after overflow = %d, want 0", got32)
+	}
 }
 
 func TestByteFrame_SequentialWrites(t *testing.T) {
diff --git a/network/binpacket/msg_bin_mail_notify.go b/network/binpacket/msg_bin_mail_notify.go
index a0e6a9dd5..c0e74e5ee 100644
--- a/network/binpacket/msg_bin_mail_notify.go
+++ b/network/binpacket/msg_bin_mail_notify.go
@@ -1,6 +1,8 @@
 package binpacket
 
 import (
+	"fmt"
+
 	"erupe-ce/common/byteframe"
 	"erupe-ce/common/stringsupport"
 	"erupe-ce/network"
@@ -13,7 +15,7 @@ type MsgBinMailNotify struct {
 
 // Parse parses the packet from binary.
 func (m MsgBinMailNotify) Parse(bf *byteframe.ByteFrame) error {
-	panic("implement me")
+	return fmt.Errorf("MsgBinMailNotify.Parse: not implemented")
 }
 
 // Build builds a binary packet from the current data.
diff --git a/network/mhfpacket/msg_sys_get_stage_binary.go b/network/mhfpacket/msg_sys_get_stage_binary.go
index c2da50122..a67aeb6d0 100644
--- a/network/mhfpacket/msg_sys_get_stage_binary.go
+++ b/network/mhfpacket/msg_sys_get_stage_binary.go
@@ -1,6 +1,8 @@
 package mhfpacket
 
 import (
+	"fmt"
+
 	"erupe-ce/common/byteframe"
 	"erupe-ce/network"
 	"erupe-ce/network/clientctx"
@@ -33,5 +35,5 @@ func (m *MsgSysGetStageBinary) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Cli
 
 // Build builds a binary packet from the current data.
 func (m *MsgSysGetStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
-	panic("Not implemented")
+	return fmt.Errorf("MsgSysGetStageBinary.Build: not implemented")
 }
diff --git a/network/mhfpacket/msg_sys_move_stage.go b/network/mhfpacket/msg_sys_move_stage.go
index 25d767da8..119b82e6f 100644
--- a/network/mhfpacket/msg_sys_move_stage.go
+++ b/network/mhfpacket/msg_sys_move_stage.go
@@ -1,8 +1,10 @@
 package mhfpacket
 
 import (
-	"erupe-ce/common/byteframe"
+	"fmt"
+
 	"erupe-ce/common/bfutil"
+	"erupe-ce/common/byteframe"
 	"erupe-ce/network"
 	"erupe-ce/network/clientctx"
 )
@@ -31,5 +33,5 @@ func (m *MsgSysMoveStage) Parse(bf *byteframe.ByteFrame, ctx *clientctx.ClientCo
 
 // Build builds a binary packet from the current data.
 func (m *MsgSysMoveStage) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
-	panic("Not implemented")
+	return fmt.Errorf("MsgSysMoveStage.Build: not implemented")
 }
diff --git a/network/mhfpacket/msg_sys_set_stage_binary.go b/network/mhfpacket/msg_sys_set_stage_binary.go
index 79832c7bb..ec256f043 100644
--- a/network/mhfpacket/msg_sys_set_stage_binary.go
+++ b/network/mhfpacket/msg_sys_set_stage_binary.go
@@ -1,6 +1,8 @@
 package mhfpacket
 
 import (
+	"fmt"
+
 	"erupe-ce/common/byteframe"
 	"erupe-ce/network"
 	"erupe-ce/network/clientctx"
@@ -32,5 +34,5 @@ func (m *MsgSysSetStageBinary) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Cli
 
 // Build builds a binary packet from the current data.
 func (m *MsgSysSetStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
-	panic("Not implemented")
+	return fmt.Errorf("MsgSysSetStageBinary.Build: not implemented")
 }
diff --git a/network/mhfpacket/msg_sys_wait_stage_binary.go b/network/mhfpacket/msg_sys_wait_stage_binary.go
index 5127e53de..7c3bcc773 100644
--- a/network/mhfpacket/msg_sys_wait_stage_binary.go
+++ b/network/mhfpacket/msg_sys_wait_stage_binary.go
@@ -1,6 +1,8 @@
 package mhfpacket
 
 import (
+	"fmt"
+
 	"erupe-ce/common/byteframe"
 	"erupe-ce/network"
 	"erupe-ce/network/clientctx"
@@ -33,5 +35,5 @@ func (m *MsgSysWaitStageBinary) Parse(bf *byteframe.ByteFrame, ctx *clientctx.Cl
 
 // Build builds a binary packet from the current data.
 func (m *MsgSysWaitStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
-	panic("Not implemented")
+	return fmt.Errorf("MsgSysWaitStageBinary.Build: not implemented")
 }
diff --git a/server/channelserver/sys_session.go b/server/channelserver/sys_session.go
index 701c7714e..926947e9f 100644
--- a/server/channelserver/sys_session.go
+++ b/server/channelserver/sys_session.go
@@ -256,6 +256,14 @@ func (s *Session) handlePacketGroup(pktGroup []byte) {
 		)
 		return
 	}
+	if bf.Err() != nil {
+		s.logger.Warn("Malformed packet (read overflow during parse)",
+			zap.String("name", s.Name),
+			zap.Stringer("opcode", opcode),
+			zap.Error(bf.Err()),
+		)
+		return
+	}
 	// Handle the packet.
 	handler, ok := s.server.handlerTable[opcode]
 	if !ok {