Skip to content

Commit fbab4b8

Browse files
committed
codec: msgpack: enforce max container length of 2^32-1
Will not throw an error if the cotainer length exceeds messagepack limit instead of doing best effort (which is wrong). Fixes #433
1 parent 71366c7 commit fbab4b8

4 files changed

Lines changed: 57 additions & 3 deletions

File tree

codec/msgpack.base.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ const (
6161
mpNegFixNumMax byte = 0xff
6262
)
6363

64+
const mpMaxLen = 1<<32 - 1 // math.MaxUint32
65+
const mpMaxLenOverflowErrorMsgPrefix = "msgpack container length exceeds max of 2^32-1"
66+
6467
var mpTimeExtTag int8 = -1
6568
var mpTimeExtTagU = uint8(mpTimeExtTag)
6669

codec/msgpack.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,9 +293,11 @@ func (e *msgpackEncDriver[T]) writeContainerLen(ct msgpackContainerType, l int)
293293
} else if l < 65536 {
294294
e.w.writen1(ct.b16)
295295
e.w.writen2(bigen.PutUint16(uint16(l)))
296-
} else {
296+
} else if l <= mpMaxLen {
297297
e.w.writen1(ct.b32)
298298
e.w.writen4(bigen.PutUint32(uint32(l)))
299+
} else {
300+
halt.errorf("%s: %d", mpMaxLenOverflowErrorMsgPrefix, l)
299301
}
300302
}
301303

codec/msgpack.mono.generated.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3353,9 +3353,11 @@ func (e *msgpackEncDriverBytes) writeContainerLen(ct msgpackContainerType, l int
33533353
} else if l < 65536 {
33543354
e.w.writen1(ct.b16)
33553355
e.w.writen2(bigen.PutUint16(uint16(l)))
3356-
} else {
3356+
} else if l <= mpMaxLen {
33573357
e.w.writen1(ct.b32)
33583358
e.w.writen4(bigen.PutUint32(uint32(l)))
3359+
} else {
3360+
halt.errorf("%s: %d", mpMaxLenOverflowErrorMsgPrefix, l)
33593361
}
33603362
}
33613363

@@ -7367,9 +7369,11 @@ func (e *msgpackEncDriverIO) writeContainerLen(ct msgpackContainerType, l int) {
73677369
} else if l < 65536 {
73687370
e.w.writen1(ct.b16)
73697371
e.w.writen2(bigen.PutUint16(uint16(l)))
7370-
} else {
7372+
} else if l <= mpMaxLen {
73717373
e.w.writen1(ct.b32)
73727374
e.w.writen4(bigen.PutUint32(uint32(l)))
7375+
} else {
7376+
halt.errorf("%s: %d", mpMaxLenOverflowErrorMsgPrefix, l)
73737377
}
73747378
}
73757379

codec/msgpack_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"net/rpc"
1313
"os/exec"
1414
"strconv"
15+
"strings"
1516
"testing"
1617
"time"
1718
)
@@ -108,6 +109,50 @@ func doTestMsgpackDecodeMapAndExtSizeMismatch(t *testing.T, h Handle) {
108109
// fn(t, b, &s)
109110
}
110111

112+
func doTestMsgpackIntOverflow(t *testing.T, h Handle) {
113+
defer testSetup(t, &h)()
114+
if cpu32Bit {
115+
t.Skip("test skipped on 32-bit machine")
116+
}
117+
if testing.Short() {
118+
t.Skip("skipping test in short mode")
119+
}
120+
overflowString := "\xa7input_b\xd9\x12HACKER OVERWRITTEN"
121+
var sb = make([]byte, (1<<32)+len(overflowString)+64)
122+
copy(sb, overflowString)
123+
overflowString = stringView(sb)
124+
// sb.Grow((1 << 32) + len(overflowString) + 64)
125+
// sb.WriteString(overflowString)
126+
// overflowString = sb.String()
127+
128+
type sampleT struct {
129+
A string
130+
B string
131+
}
132+
133+
var v0, v1 sampleT
134+
var b []byte
135+
var err error
136+
137+
v0 = sampleT{"normal", "normal value"}
138+
b = testMarshalErr(v0, h, t, "encode-normal")
139+
testUnmarshalErr(&v1, b, h, t, "decode")
140+
testDeepEqualErr(v0, v1, t, "compare")
141+
142+
v0 = sampleT{"hacker", overflowString}
143+
b, err = testMarshal(v0, h)
144+
if err == nil || !strings.Contains(err.Error(), mpMaxLenOverflowErrorMsgPrefix) {
145+
t.Fatalf("expected error that len exceeds maximum")
146+
}
147+
// v1 = sampleT{}
148+
// testUnmarshalErr(&v1, b, h, t, "decode")
149+
// testDeepEqualErr(v0, v1, t, "compare")
150+
}
151+
152+
func TestMsgpackIntOverflow(t *testing.T) {
153+
doTestMsgpackIntOverflow(t, testMsgpackH)
154+
}
155+
111156
func TestMsgpackCodecsTable(t *testing.T) {
112157
doTestCodecTableOne(t, testMsgpackH)
113158
}

0 commit comments

Comments
 (0)