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.
This commit is contained in:
Houmgaor
2026-02-20 20:00:54 +01:00
parent 7c444b023b
commit ac59188488
8 changed files with 105 additions and 15 deletions

View File

@@ -9,16 +9,21 @@ import (
"bytes" "bytes"
"encoding/binary" "encoding/binary"
"errors" "errors"
"fmt"
"io" "io"
"math" "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. // ByteFrame is a struct for reading and writing raw byte data.
type ByteFrame struct { type ByteFrame struct {
index uint index uint
usedSize uint usedSize uint
buf []byte buf []byte
byteOrder binary.ByteOrder byteOrder binary.ByteOrder
err error // sticky error set on read overflow
} }
// NewByteFrame creates a new ByteFrame with valid default values. // NewByteFrame creates a new ByteFrame with valid default values.
@@ -92,7 +97,14 @@ func (b *ByteFrame) rprologue(size uint) {
} }
func (b *ByteFrame) rerr() { 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) // Seek (implements the io.Seeker interface)
@@ -247,8 +259,12 @@ func (b *ByteFrame) WriteNullTerminatedBytes(x []byte) {
// ReadUint8 reads a uint8 at the current index. // ReadUint8 reads a uint8 at the current index.
func (b *ByteFrame) ReadUint8() (x uint8) { func (b *ByteFrame) ReadUint8() (x uint8) {
if b.err != nil {
return 0
}
if !b.rcheck(1) { if !b.rcheck(1) {
b.rerr() b.rerr()
return 0
} }
x = uint8(b.buf[b.index]) x = uint8(b.buf[b.index])
b.rprologue(1) b.rprologue(1)
@@ -265,8 +281,12 @@ func (b *ByteFrame) ReadBool() (x bool) {
// ReadUint16 reads a uint16 at the current index. // ReadUint16 reads a uint16 at the current index.
func (b *ByteFrame) ReadUint16() (x uint16) { func (b *ByteFrame) ReadUint16() (x uint16) {
if b.err != nil {
return 0
}
if !b.rcheck(2) { if !b.rcheck(2) {
b.rerr() b.rerr()
return 0
} }
x = b.byteOrder.Uint16(b.buf[b.index:]) x = b.byteOrder.Uint16(b.buf[b.index:])
b.rprologue(2) b.rprologue(2)
@@ -275,8 +295,12 @@ func (b *ByteFrame) ReadUint16() (x uint16) {
// ReadUint32 reads a uint32 at the current index. // ReadUint32 reads a uint32 at the current index.
func (b *ByteFrame) ReadUint32() (x uint32) { func (b *ByteFrame) ReadUint32() (x uint32) {
if b.err != nil {
return 0
}
if !b.rcheck(4) { if !b.rcheck(4) {
b.rerr() b.rerr()
return 0
} }
x = b.byteOrder.Uint32(b.buf[b.index:]) x = b.byteOrder.Uint32(b.buf[b.index:])
b.rprologue(4) b.rprologue(4)
@@ -285,8 +309,12 @@ func (b *ByteFrame) ReadUint32() (x uint32) {
// ReadUint64 reads a uint64 at the current index. // ReadUint64 reads a uint64 at the current index.
func (b *ByteFrame) ReadUint64() (x uint64) { func (b *ByteFrame) ReadUint64() (x uint64) {
if b.err != nil {
return 0
}
if !b.rcheck(8) { if !b.rcheck(8) {
b.rerr() b.rerr()
return 0
} }
x = b.byteOrder.Uint64(b.buf[b.index:]) x = b.byteOrder.Uint64(b.buf[b.index:])
b.rprologue(8) b.rprologue(8)
@@ -295,8 +323,12 @@ func (b *ByteFrame) ReadUint64() (x uint64) {
// ReadInt8 reads a int8 at the current index. // ReadInt8 reads a int8 at the current index.
func (b *ByteFrame) ReadInt8() (x int8) { func (b *ByteFrame) ReadInt8() (x int8) {
if b.err != nil {
return 0
}
if !b.rcheck(1) { if !b.rcheck(1) {
b.rerr() b.rerr()
return 0
} }
x = int8(b.buf[b.index]) x = int8(b.buf[b.index])
b.rprologue(1) b.rprologue(1)
@@ -305,8 +337,12 @@ func (b *ByteFrame) ReadInt8() (x int8) {
// ReadInt16 reads a int16 at the current index. // ReadInt16 reads a int16 at the current index.
func (b *ByteFrame) ReadInt16() (x int16) { func (b *ByteFrame) ReadInt16() (x int16) {
if b.err != nil {
return 0
}
if !b.rcheck(2) { if !b.rcheck(2) {
b.rerr() b.rerr()
return 0
} }
x = int16(b.byteOrder.Uint16(b.buf[b.index:])) x = int16(b.byteOrder.Uint16(b.buf[b.index:]))
b.rprologue(2) b.rprologue(2)
@@ -315,8 +351,12 @@ func (b *ByteFrame) ReadInt16() (x int16) {
// ReadInt32 reads a int32 at the current index. // ReadInt32 reads a int32 at the current index.
func (b *ByteFrame) ReadInt32() (x int32) { func (b *ByteFrame) ReadInt32() (x int32) {
if b.err != nil {
return 0
}
if !b.rcheck(4) { if !b.rcheck(4) {
b.rerr() b.rerr()
return 0
} }
x = int32(b.byteOrder.Uint32(b.buf[b.index:])) x = int32(b.byteOrder.Uint32(b.buf[b.index:]))
b.rprologue(4) b.rprologue(4)
@@ -325,8 +365,12 @@ func (b *ByteFrame) ReadInt32() (x int32) {
// ReadInt64 reads a int64 at the current index. // ReadInt64 reads a int64 at the current index.
func (b *ByteFrame) ReadInt64() (x int64) { func (b *ByteFrame) ReadInt64() (x int64) {
if b.err != nil {
return 0
}
if !b.rcheck(8) { if !b.rcheck(8) {
b.rerr() b.rerr()
return 0
} }
x = int64(b.byteOrder.Uint64(b.buf[b.index:])) x = int64(b.byteOrder.Uint64(b.buf[b.index:]))
b.rprologue(8) b.rprologue(8)
@@ -335,8 +379,12 @@ func (b *ByteFrame) ReadInt64() (x int64) {
// ReadFloat32 reads a float32 at the current index. // ReadFloat32 reads a float32 at the current index.
func (b *ByteFrame) ReadFloat32() (x float32) { func (b *ByteFrame) ReadFloat32() (x float32) {
if b.err != nil {
return 0
}
if !b.rcheck(4) { if !b.rcheck(4) {
b.rerr() b.rerr()
return 0
} }
x = math.Float32frombits(b.byteOrder.Uint32(b.buf[b.index:])) x = math.Float32frombits(b.byteOrder.Uint32(b.buf[b.index:]))
b.rprologue(4) b.rprologue(4)
@@ -345,8 +393,12 @@ func (b *ByteFrame) ReadFloat32() (x float32) {
// ReadFloat64 reads a float64 at the current index. // ReadFloat64 reads a float64 at the current index.
func (b *ByteFrame) ReadFloat64() (x float64) { func (b *ByteFrame) ReadFloat64() (x float64) {
if b.err != nil {
return 0
}
if !b.rcheck(8) { if !b.rcheck(8) {
b.rerr() b.rerr()
return 0
} }
x = math.Float64frombits(b.byteOrder.Uint64(b.buf[b.index:])) x = math.Float64frombits(b.byteOrder.Uint64(b.buf[b.index:]))
b.rprologue(8) b.rprologue(8)
@@ -355,8 +407,12 @@ func (b *ByteFrame) ReadFloat64() (x float64) {
// ReadBytes reads `size` many bytes at the current index. // ReadBytes reads `size` many bytes at the current index.
func (b *ByteFrame) ReadBytes(size uint) (x []byte) { func (b *ByteFrame) ReadBytes(size uint) (x []byte) {
if b.err != nil {
return nil
}
if !b.rcheck(size) { if !b.rcheck(size) {
b.rerr() b.rerr()
return nil
} }
x = b.buf[b.index : b.index+size] x = b.buf[b.index : b.index+size]
b.rprologue(size) b.rprologue(size)

View File

@@ -3,6 +3,7 @@ package byteframe
import ( import (
"bytes" "bytes"
"encoding/binary" "encoding/binary"
"errors"
"io" "io"
"math" "math"
"testing" "testing"
@@ -430,18 +431,33 @@ func TestByteFrame_BufferGrowth(t *testing.T) {
} }
} }
func TestByteFrame_ReadPanic(t *testing.T) { func TestByteFrame_ReadOverflowSetsError(t *testing.T) {
defer func() {
if r := recover(); r == nil {
t.Error("Reading beyond buffer should panic")
}
}()
bf := NewByteFrame() bf := NewByteFrame()
bf.WriteUint8(0x01) bf.WriteUint8(0x01)
_, _ = bf.Seek(0, io.SeekStart) _, _ = bf.Seek(0, io.SeekStart)
bf.ReadUint8() 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) { func TestByteFrame_SequentialWrites(t *testing.T) {

View File

@@ -1,6 +1,8 @@
package binpacket package binpacket
import ( import (
"fmt"
"erupe-ce/common/byteframe" "erupe-ce/common/byteframe"
"erupe-ce/common/stringsupport" "erupe-ce/common/stringsupport"
"erupe-ce/network" "erupe-ce/network"
@@ -13,7 +15,7 @@ type MsgBinMailNotify struct {
// Parse parses the packet from binary. // Parse parses the packet from binary.
func (m MsgBinMailNotify) Parse(bf *byteframe.ByteFrame) error { 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. // Build builds a binary packet from the current data.

View File

@@ -1,6 +1,8 @@
package mhfpacket package mhfpacket
import ( import (
"fmt"
"erupe-ce/common/byteframe" "erupe-ce/common/byteframe"
"erupe-ce/network" "erupe-ce/network"
"erupe-ce/network/clientctx" "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. // Build builds a binary packet from the current data.
func (m *MsgSysGetStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error { func (m *MsgSysGetStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
panic("Not implemented") return fmt.Errorf("MsgSysGetStageBinary.Build: not implemented")
} }

View File

@@ -1,8 +1,10 @@
package mhfpacket package mhfpacket
import ( import (
"erupe-ce/common/byteframe" "fmt"
"erupe-ce/common/bfutil" "erupe-ce/common/bfutil"
"erupe-ce/common/byteframe"
"erupe-ce/network" "erupe-ce/network"
"erupe-ce/network/clientctx" "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. // Build builds a binary packet from the current data.
func (m *MsgSysMoveStage) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error { func (m *MsgSysMoveStage) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
panic("Not implemented") return fmt.Errorf("MsgSysMoveStage.Build: not implemented")
} }

View File

@@ -1,6 +1,8 @@
package mhfpacket package mhfpacket
import ( import (
"fmt"
"erupe-ce/common/byteframe" "erupe-ce/common/byteframe"
"erupe-ce/network" "erupe-ce/network"
"erupe-ce/network/clientctx" "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. // Build builds a binary packet from the current data.
func (m *MsgSysSetStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error { func (m *MsgSysSetStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
panic("Not implemented") return fmt.Errorf("MsgSysSetStageBinary.Build: not implemented")
} }

View File

@@ -1,6 +1,8 @@
package mhfpacket package mhfpacket
import ( import (
"fmt"
"erupe-ce/common/byteframe" "erupe-ce/common/byteframe"
"erupe-ce/network" "erupe-ce/network"
"erupe-ce/network/clientctx" "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. // Build builds a binary packet from the current data.
func (m *MsgSysWaitStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error { func (m *MsgSysWaitStageBinary) Build(bf *byteframe.ByteFrame, ctx *clientctx.ClientContext) error {
panic("Not implemented") return fmt.Errorf("MsgSysWaitStageBinary.Build: not implemented")
} }

View File

@@ -256,6 +256,14 @@ func (s *Session) handlePacketGroup(pktGroup []byte) {
) )
return 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. // Handle the packet.
handler, ok := s.server.handlerTable[opcode] handler, ok := s.server.handlerTable[opcode]
if !ok { if !ok {