Commit c884bfd5 authored by Kirill Smelkov's avatar Kirill Smelkov

X neo/protogen: Catch length checks overflows on decode

For example a list is encoded as

	l        u32
	[l]item  itemType

on decode len is read from data stream and for rest of data len(data) is
checked to be < l*sizeof(item).

However since l is u32 and sizeof(item) is just number the result of `l
* sizeof(item)` has also u32 type. However it could overflow e.g. for

	l		= 0x20000000
	sizeof(item)	= 8

with the l*sizeof(item) being = u32(0)	(exactly zero) -> oops.

Avoid the problem by doing all checking arithmetics with u64 ints.
parent d7c9395b
...@@ -244,7 +244,7 @@ func (a *Address) neoEncode(b []byte) int { ...@@ -244,7 +244,7 @@ func (a *Address) neoEncode(b []byte) int {
return n return n
} }
func (a *Address) neoDecode(b []byte) (uint32, bool) { func (a *Address) neoDecode(b []byte) (uint64, bool) {
n, ok := string_neoDecode(&a.Host, b) n, ok := string_neoDecode(&a.Host, b)
if !ok { if !ok {
return 0, false return 0, false
...@@ -288,7 +288,7 @@ func (t IdTime) neoEncode(b []byte) int { ...@@ -288,7 +288,7 @@ func (t IdTime) neoEncode(b []byte) int {
return 8 return 8
} }
func (t *IdTime) neoDecode(data []byte) (uint32, bool) { func (t *IdTime) neoDecode(data []byte) (uint64, bool) {
if len(data) < 8 { if len(data) < 8 {
return 0, false return 0, false
} }
...@@ -1055,7 +1055,7 @@ type Truncate struct { ...@@ -1055,7 +1055,7 @@ type Truncate struct {
type customCodec interface { type customCodec interface {
neoEncodedLen() int neoEncodedLen() int
neoEncode(buf []byte) (nwrote int) neoEncode(buf []byte) (nwrote int)
neoDecode(data []byte) (nread uint32, ok bool) // XXX uint32 or int here? neoDecode(data []byte) (nread uint64, ok bool) // XXX uint64 or int here?
} }
func byte2bool(b byte) bool { func byte2bool(b byte) bool {
...@@ -1104,16 +1104,16 @@ func string_neoEncode(s string, data []byte) int { ...@@ -1104,16 +1104,16 @@ func string_neoEncode(s string, data []byte) int {
return 4 + l return 4 + l
} }
func string_neoDecode(sp *string, data []byte) (nread uint32, ok bool) { func string_neoDecode(sp *string, data []byte) (nread uint64, ok bool) {
if len(data) < 4 { if len(data) < 4 {
return 0, false return 0, false
} }
l := binary.BigEndian.Uint32(data) l := binary.BigEndian.Uint32(data)
data = data[4:] data = data[4:]
if uint32(len(data)) < l { if uint64(len(data)) < uint64(l) {
return 0, false return 0, false
} }
*sp = string(data[:l]) *sp = string(data[:l])
return 4 + l, true return 4 + uint64(l), true
} }
...@@ -751,7 +751,7 @@ func (d *decoder) overflowCheck() { ...@@ -751,7 +751,7 @@ func (d *decoder) overflowCheck() {
//d.bufDone.emit("// overflow check point") //d.bufDone.emit("// overflow check point")
if !d.overflow.checkSize.IsZero() { if !d.overflow.checkSize.IsZero() {
d.bufDone.emit("if uint32(len(data)) < %v { goto overflow }", &d.overflow.checkSize) d.bufDone.emit("if uint64(len(data)) < %v { goto overflow }", &d.overflow.checkSize)
// if size for overflow check was only numeric - just // if size for overflow check was only numeric - just
// accumulate it at generation time // accumulate it at generation time
...@@ -816,7 +816,7 @@ func (d *decoder) generatedCode() string { ...@@ -816,7 +816,7 @@ func (d *decoder) generatedCode() string {
// prologue // prologue
code.emit("func (%s *%s) neoMsgDecode(data []byte) (int, error) {", d.recvName, d.typeName) code.emit("func (%s *%s) neoMsgDecode(data []byte) (int, error) {", d.recvName, d.typeName)
if d.varUsed["nread"] { if d.varUsed["nread"] {
code.emit("var %v uint32", d.var_("nread")) code.emit("var %v uint64", d.var_("nread"))
} }
code.Write(d.bufDone.Bytes()) code.Write(d.bufDone.Bytes())
...@@ -824,6 +824,10 @@ func (d *decoder) generatedCode() string { ...@@ -824,6 +824,10 @@ func (d *decoder) generatedCode() string {
// epilogue // epilogue
retexpr := fmt.Sprintf("%v", d.nread) retexpr := fmt.Sprintf("%v", d.nread)
if d.varUsed["nread"] { if d.varUsed["nread"] {
// casting nread to int is ok even on 32 bit arches:
// if nread would overflow 32 bits it would be caught earlier,
// because on 32 bit arch len(data) is also 32 bit and in generated
// code len(data) is checked first to be less than encoded message.
retexpr += fmt.Sprintf(" + int(%v)", d.var_("nread")) retexpr += fmt.Sprintf(" + int(%v)", d.var_("nread"))
} }
code.emit("return %v, nil", retexpr) code.emit("return %v, nil", retexpr)
...@@ -918,7 +922,7 @@ func (d *decoder) genSlice1(assignto string, typ types.Type) { ...@@ -918,7 +922,7 @@ func (d *decoder) genSlice1(assignto string, typ types.Type) {
d.resetPos() d.resetPos()
d.overflowCheck() d.overflowCheck()
d.overflow.AddExpr("l") d.overflow.AddExpr("uint64(l)")
switch t := typ.(type) { switch t := typ.(type) {
case *types.Basic: case *types.Basic:
...@@ -957,7 +961,7 @@ func (d *decoder) genBuf(path string) { ...@@ -957,7 +961,7 @@ func (d *decoder) genBuf(path string) {
d.resetPos() d.resetPos()
d.overflowCheck() d.overflowCheck()
d.overflow.AddExpr("l") d.overflow.AddExpr("uint64(l)")
// TODO eventually do not copy but reference original // TODO eventually do not copy but reference original
d.emit("%v= mem.BufAlloc(int(l))", path) d.emit("%v= mem.BufAlloc(int(l))", path)
...@@ -1022,7 +1026,7 @@ func (d *decoder) genSlice(assignto string, typ *types.Slice, obj types.Object) ...@@ -1022,7 +1026,7 @@ func (d *decoder) genSlice(assignto string, typ *types.Slice, obj types.Object)
elemSize, elemFixed := typeSizeFixed(typ.Elem()) elemSize, elemFixed := typeSizeFixed(typ.Elem())
if elemFixed { if elemFixed {
d.overflowCheck() d.overflowCheck()
d.overflow.AddExpr("l * %v", elemSize) d.overflow.AddExpr("uint64(l) * %v", elemSize)
d.overflow.PushChecked(true) d.overflow.PushChecked(true)
defer d.overflow.PopChecked() defer d.overflow.PopChecked()
} }
...@@ -1036,7 +1040,7 @@ func (d *decoder) genSlice(assignto string, typ *types.Slice, obj types.Object) ...@@ -1036,7 +1040,7 @@ func (d *decoder) genSlice(assignto string, typ *types.Slice, obj types.Object)
d.resetPos() d.resetPos()
d.emit("}") d.emit("}")
d.overflowCheckLoopExit("l") d.overflowCheckLoopExit("uint64(l)")
d.emit("}") d.emit("}")
} }
...@@ -1107,7 +1111,7 @@ func (d *decoder) genMap(assignto string, typ *types.Map, obj types.Object) { ...@@ -1107,7 +1111,7 @@ func (d *decoder) genMap(assignto string, typ *types.Map, obj types.Object) {
elemSize, elemFixed := typeSizeFixed(typ.Elem()) elemSize, elemFixed := typeSizeFixed(typ.Elem())
if keyFixed && elemFixed { if keyFixed && elemFixed {
d.overflowCheck() d.overflowCheck()
d.overflow.AddExpr("l * %v", keySize+elemSize) d.overflow.AddExpr("uint64(l) * %v", keySize+elemSize)
d.overflow.PushChecked(true) d.overflow.PushChecked(true)
defer d.overflow.PopChecked() defer d.overflow.PopChecked()
} }
...@@ -1133,7 +1137,7 @@ func (d *decoder) genMap(assignto string, typ *types.Map, obj types.Object) { ...@@ -1133,7 +1137,7 @@ func (d *decoder) genMap(assignto string, typ *types.Map, obj types.Object) {
d.resetPos() d.resetPos()
d.emit("}") d.emit("}")
d.overflowCheckLoopExit("l") d.overflowCheckLoopExit("uint64(l)")
d.emit("}") d.emit("}")
} }
......
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment