From 85ca17967cf11654118d84eb6e1acc4ab37bb851 Mon Sep 17 00:00:00 2001 From: Andreas Krennmair Date: Wed, 29 Jul 2015 17:39:26 +0200 Subject: [PATCH 1/4] Add Fuzz function as required by go-fuzz --- fuzz.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 fuzz.go diff --git a/fuzz.go b/fuzz.go new file mode 100644 index 00000000..bf7c7689 --- /dev/null +++ b/fuzz.go @@ -0,0 +1,16 @@ +// +build gofuzz +package amqp + +import "bytes" + +func Fuzz(data []byte) int { + r := reader{bytes.NewReader(data)} + frame, err := r.ReadFrame() + if err != nil { + if frame != nil { + panic("frame is not nil") + } + return 0 + } + return 1 +} From c412303a2ce73998930ea6fc474cdce8f3d5588c Mon Sep 17 00:00:00 2001 From: Andreas Krennmair Date: Wed, 29 Jul 2015 17:40:35 +0200 Subject: [PATCH 2/4] Establish baseline with tests based on crashers found by go-fuzz --- read_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 read_test.go diff --git a/read_test.go b/read_test.go new file mode 100644 index 00000000..bb0e30f0 --- /dev/null +++ b/read_test.go @@ -0,0 +1,22 @@ +package amqp + +import ( + "strings" + "testing" +) + +func TestGoFuzzCrashers(t *testing.T) { + testData := []string{ + "\b000000", + "\x02\x16\x10�[��\t\xbdui�" + "\x10\x01\x00\xff\xbf\xef\xbfサn\x99\x00\x10r", + "\x0300\x00\x00\x00\x040000", + } + + for idx, testStr := range testData { + r := reader{strings.NewReader(testStr)} + frame, err := r.ReadFrame() + if err != nil && frame != nil { + t.Errorf("%d. frame is not nil: %#v err = %v", idx, frame, err) + } + } +} From 4e49d12d9056b5e423aee9b39a58a6eca5a296c4 Mon Sep 17 00:00:00 2001 From: Andreas Krennmair Date: Wed, 29 Jul 2015 17:51:06 +0200 Subject: [PATCH 3/4] Indicate an error instead of a panic if a heartbeat contains a payload --- read.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/read.go b/read.go index 2ef7a9e4..375bbebc 100644 --- a/read.go +++ b/read.go @@ -8,6 +8,7 @@ package amqp import ( "bytes" "encoding/binary" + "errors" "io" "time" ) @@ -431,13 +432,15 @@ func (me *reader) parseBodyFrame(channel uint16, size uint32) (frame frame, err return bf, nil } +var errHeartbeatPayload = errors.New("Heartbeats should not have a payload") + func (me *reader) parseHeartbeatFrame(channel uint16, size uint32) (frame frame, err error) { hf := &heartbeatFrame{ ChannelId: channel, } if size > 0 { - panic("Heartbeats should not have a payload") + return nil, errHeartbeatPayload } return hf, nil From eeed29bef608a7af467394748866346b70888be6 Mon Sep 17 00:00:00 2001 From: Andreas Krennmair Date: Wed, 29 Jul 2015 17:54:10 +0200 Subject: [PATCH 4/4] When reading a body frame, return either a frame or an error, but never both --- read.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/read.go b/read.go index 375bbebc..74e90ef8 100644 --- a/read.go +++ b/read.go @@ -67,7 +67,7 @@ func (me *reader) ReadFrame() (frame frame, err error) { case frameBody: if frame, err = me.parseBodyFrame(channel, size); err != nil { - return + return nil, err } case frameHeartbeat: @@ -80,7 +80,7 @@ func (me *reader) ReadFrame() (frame frame, err error) { } if _, err = io.ReadFull(me.r, scratch[:1]); err != nil { - return + return nil, err } if scratch[0] != frameEnd { @@ -426,7 +426,7 @@ func (me *reader) parseBodyFrame(channel uint16, size uint32) (frame frame, err } if _, err = io.ReadFull(me.r, bf.Body); err != nil { - return + return nil, err } return bf, nil