Skip to content

Commit fa9f074

Browse files
Sean-Derclaude
andcommitted
Fix panic on short XOR-MAPPED-ADDRESS value
XORMappedAddress.GetFromAs read value[0:2] to decode the address family before validating the attribute value length. A malformed STUN message carrying an XOR-MAPPED-ADDRESS attribute with a 0-length value at the end of a tightly allocated buffer triggered a slice-bounds runtime panic, since the value slice had no spare capacity to re-slice into. Move the length check ahead of the first read so a short value returns io.ErrUnexpectedEOF instead of panicking. This is remotely triggerable on normal STUN/ICE Binding-response parsing paths. Reported by Karolina GORNA Reported-By: karolina.gorna@ledger.com Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 01aa5b8 commit fa9f074

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

xoraddr.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ func (a *XORMappedAddress) GetFromAs(msg *Message, attr AttrType) error {
9191
if err != nil {
9292
return err
9393
}
94+
if len(value) <= 4 {
95+
return io.ErrUnexpectedEOF
96+
}
9497
family := bin.Uint16(value[0:2])
9598
if family != familyIPv6 && family != familyIPv4 {
9699
return newDecodeErr("xor-mapped address", "family",
@@ -111,9 +114,6 @@ func (a *XORMappedAddress) GetFromAs(msg *Message, attr AttrType) error {
111114
}
112115
}
113116

114-
if len(value) <= 4 {
115-
return io.ErrUnexpectedEOF
116-
}
117117
if err := CheckOverflow(attr, len(value[4:]), len(a.IP)); err != nil {
118118
return err
119119
}

xoraddr_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,22 @@ func TestXORMappedAddress_GetFrom(t *testing.T) {
6969
addr := new(XORMappedAddress)
7070
assert.True(t, IsAttrSizeOverflow(addr.GetFrom(m)), "GetFrom should return *AttrOverflowErr")
7171
})
72+
t.Run("ShortValue", func(t *testing.T) {
73+
// A zero-length XOR-MAPPED-ADDRESS value at the end of a tightly
74+
// allocated buffer must not panic when reading the address family.
75+
raw := []byte{
76+
0x01, 0x01, 0x00, 0x04, // type=Binding success, length=4
77+
0x21, 0x12, 0xA4, 0x42, // magic cookie
78+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // transaction ID
79+
0x00, 0x20, 0x00, 0x00, // XOR-MAPPED-ADDRESS, length=0
80+
}
81+
// Cap == len so the decoded value slice has no spare capacity.
82+
m := New()
83+
m.Raw = raw[:len(raw):len(raw)]
84+
assert.NoError(t, m.Decode())
85+
addr := new(XORMappedAddress)
86+
assert.ErrorIs(t, addr.GetFrom(m), io.ErrUnexpectedEOF, "short value should return io.ErrUnexpectedEOF, not panic")
87+
})
7288
}
7389

7490
func TestXORMappedAddress_GetFrom_Invalid(t *testing.T) {

0 commit comments

Comments
 (0)