From b39aaeac97c5b1ed1eadd8ba0d1a05c27a7c71ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rio=20Freitas?= Date: Mon, 28 Mar 2016 17:51:47 +0900 Subject: [PATCH 1/3] safer and fail-faster reading of bogus frames --- .travis.yml | 2 +- read.go | 54 +++++++++++++++++++++++++++++++++++----------------- read_test.go | 1 + 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index f1c275a2..aff0400a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,4 +11,4 @@ services: env: - AMQP_URL=amqp://guest:guest@127.0.0.1:5672/ GOMAXPROCS=2 -script: go test -v -tags integration ./... +script: go test -v -race -tags integration ./... diff --git a/read.go b/read.go index 74e90ef8..f58178d1 100644 --- a/read.go +++ b/read.go @@ -6,6 +6,7 @@ package amqp import ( + "bufio" "bytes" "encoding/binary" "errors" @@ -90,15 +91,35 @@ func (me *reader) ReadFrame() (frame frame, err error) { return } -func readShortstr(r io.Reader) (v string, err error) { +func readN(r io.Reader, n int64) ([]byte, error) { + if n <= 1024*1024 { + if n < 0 { + return nil, bufio.ErrNegativeCount + } + b := make([]byte, n) + if _, err := io.ReadFull(r, b); err != nil { + return nil, err + } + return b, nil + } + + buf := bytes.NewBuffer(nil) + br := bufio.NewReaderSize(r, 64*1024) + if _, err := io.CopyN(buf, br, n); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +func readShortstr(r io.Reader) (string, error) { var length uint8 - if err = binary.Read(r, binary.BigEndian, &length); err != nil { - return + if err := binary.Read(r, binary.BigEndian, &length); err != nil { + return "", err } - bytes := make([]byte, length) - if _, err = io.ReadFull(r, bytes); err != nil { - return + bytes, err := readN(r, int64(length)) + if err != nil { + return "", err } return string(bytes), nil } @@ -109,9 +130,9 @@ func readLongstr(r io.Reader) (v string, err error) { return } - bytes := make([]byte, length) - if _, err = io.ReadFull(r, bytes); err != nil { - return + bytes, err := readN(r, int64(length)) + if err != nil { + return "", err } return string(bytes), nil } @@ -241,8 +262,8 @@ func readField(r io.Reader) (v interface{}, err error) { return nil, err } - value := make([]byte, len) - if _, err = io.ReadFull(r, value); err != nil { + value, err := readN(r, int64(len)) + if err != nil { return nil, err } return value, err @@ -420,15 +441,14 @@ func (me *reader) parseHeaderFrame(channel uint16, size uint32) (frame frame, er } func (me *reader) parseBodyFrame(channel uint16, size uint32) (frame frame, err error) { + body, err := readN(me.r, int64(size)) + if err != nil { + return nil, err + } bf := &bodyFrame{ ChannelId: channel, - Body: make([]byte, size), + Body: body, } - - if _, err = io.ReadFull(me.r, bf.Body); err != nil { - return nil, err - } - return bf, nil } diff --git a/read_test.go b/read_test.go index bb0e30f0..4404cdff 100644 --- a/read_test.go +++ b/read_test.go @@ -10,6 +10,7 @@ func TestGoFuzzCrashers(t *testing.T) { "\b000000", "\x02\x16\x10�[��\t\xbdui�" + "\x10\x01\x00\xff\xbf\xef\xbfｻn\x99\x00\x10r", "\x0300\x00\x00\x00\x040000", + "\x020000000000000000000" + "0\x00\x00\x000!00000000000000" + "0000000000000000000x" + "\x800000000000000000000" + "00000000000000000000" + "00000000000000000000" + "00", } for idx, testStr := range testData { From 654c98b591e42c4010aa85d7b5b2493e937de17d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rio=20Freitas?= Date: Mon, 28 Mar 2016 17:51:47 +0900 Subject: [PATCH 2/3] safer and fail-faster reading of bogus frames --- .travis.yml | 2 +- read.go | 54 +++++++++++++++++++++++++++++++++++----------------- read_test.go | 1 + 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index d2172035..7e005c7e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,4 +11,4 @@ services: env: - AMQP_URL=amqp://guest:guest@127.0.0.1:5672/ GOMAXPROCS=2 -script: go test -v -tags integration ./... +script: go test -v -race -tags integration ./... diff --git a/read.go b/read.go index 74e90ef8..f58178d1 100644 --- a/read.go +++ b/read.go @@ -6,6 +6,7 @@ package amqp import ( + "bufio" "bytes" "encoding/binary" "errors" @@ -90,15 +91,35 @@ func (me *reader) ReadFrame() (frame frame, err error) { return } -func readShortstr(r io.Reader) (v string, err error) { +func readN(r io.Reader, n int64) ([]byte, error) { + if n <= 1024*1024 { + if n < 0 { + return nil, bufio.ErrNegativeCount + } + b := make([]byte, n) + if _, err := io.ReadFull(r, b); err != nil { + return nil, err + } + return b, nil + } + + buf := bytes.NewBuffer(nil) + br := bufio.NewReaderSize(r, 64*1024) + if _, err := io.CopyN(buf, br, n); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +func readShortstr(r io.Reader) (string, error) { var length uint8 - if err = binary.Read(r, binary.BigEndian, &length); err != nil { - return + if err := binary.Read(r, binary.BigEndian, &length); err != nil { + return "", err } - bytes := make([]byte, length) - if _, err = io.ReadFull(r, bytes); err != nil { - return + bytes, err := readN(r, int64(length)) + if err != nil { + return "", err } return string(bytes), nil } @@ -109,9 +130,9 @@ func readLongstr(r io.Reader) (v string, err error) { return } - bytes := make([]byte, length) - if _, err = io.ReadFull(r, bytes); err != nil { - return + bytes, err := readN(r, int64(length)) + if err != nil { + return "", err } return string(bytes), nil } @@ -241,8 +262,8 @@ func readField(r io.Reader) (v interface{}, err error) { return nil, err } - value := make([]byte, len) - if _, err = io.ReadFull(r, value); err != nil { + value, err := readN(r, int64(len)) + if err != nil { return nil, err } return value, err @@ -420,15 +441,14 @@ func (me *reader) parseHeaderFrame(channel uint16, size uint32) (frame frame, er } func (me *reader) parseBodyFrame(channel uint16, size uint32) (frame frame, err error) { + body, err := readN(me.r, int64(size)) + if err != nil { + return nil, err + } bf := &bodyFrame{ ChannelId: channel, - Body: make([]byte, size), + Body: body, } - - if _, err = io.ReadFull(me.r, bf.Body); err != nil { - return nil, err - } - return bf, nil } diff --git a/read_test.go b/read_test.go index bb0e30f0..4404cdff 100644 --- a/read_test.go +++ b/read_test.go @@ -10,6 +10,7 @@ func TestGoFuzzCrashers(t *testing.T) { "\b000000", "\x02\x16\x10�[��\t\xbdui�" + "\x10\x01\x00\xff\xbf\xef\xbfｻn\x99\x00\x10r", "\x0300\x00\x00\x00\x040000", + "\x020000000000000000000" + "0\x00\x00\x000!00000000000000" + "0000000000000000000x" + "\x800000000000000000000" + "00000000000000000000" + "00000000000000000000" + "00", } for idx, testStr := range testData { From 3ff83b1b0ad1564066075ca08175190ea8346f0d Mon Sep 17 00:00:00 2001 From: Michael Klishin Date: Sat, 26 Nov 2016 01:42:33 +0300 Subject: [PATCH 3/3] Revert "safer and fail-faster reading of bogus frames"