From f07d4b3dbb973080e6ae067513cf47b2533c71ae Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Wed, 14 Feb 2018 22:43:49 +0100 Subject: [PATCH] AMQP: Fix panic during parse of partial message (#6384) A message with a client header consisting on a partial frame (not all data received for this frame) could result in a panic. --- CHANGELOG.asciidoc | 1 + packetbeat/protos/amqp/amqp_parser.go | 5 ++++- packetbeat/protos/amqp/amqp_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index d3283d6f41d..f587fc194fc 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -42,6 +42,7 @@ https://github.com/elastic/beats/compare/v5.6.8...5.6[Check the HEAD diff] - Fix http parse to allow to parse get request with space in the URI. {pull}5495[5495] - Fix mysql SQL parser to trim `\r` from Windows Server `SELECT\r\n\t1`. {pull}5572[5572] - Fix corruption when parsing repeated headers in an HTTP request or response. {pull}6325[6325] +- Fix panic when parsing partial AMQP messages. {pull}6384[6384] *Winlogbeat* diff --git a/packetbeat/protos/amqp/amqp_parser.go b/packetbeat/protos/amqp/amqp_parser.go index 0ec4a39c71d..00b3cd01a10 100644 --- a/packetbeat/protos/amqp/amqp_parser.go +++ b/packetbeat/protos/amqp/amqp_parser.go @@ -73,7 +73,10 @@ func isProtocolHeader(data []byte) (isHeader bool, version string) { //func to read a frame header and check if it is valid and complete func readFrameHeader(data []byte) (ret *amqpFrame, err bool) { var frame amqpFrame - + if len(data) < 8 { + logp.Warn("Partial frame header, waiting for more data") + return nil, false + } frame.size = binary.BigEndian.Uint32(data[3:7]) if len(data) < int(frame.size)+8 { logp.Warn("Frame shorter than declared size, waiting for more data") diff --git a/packetbeat/protos/amqp/amqp_test.go b/packetbeat/protos/amqp/amqp_test.go index be8d198391a..1222be56988 100644 --- a/packetbeat/protos/amqp/amqp_test.go +++ b/packetbeat/protos/amqp/amqp_test.go @@ -83,6 +83,30 @@ func TestAmqp_FrameSize(t *testing.T) { } } +// Test that the parser doesn't panic on a partial message that includes +// a client header +func TestAmqp_PartialFrameSize(t *testing.T) { + if testing.Verbose() { + logp.LogInit(logp.LOG_DEBUG, "", false, true, []string{"amqp", "amqpdetailed"}) + } + + amqp := amqpModForTests() + + //incomplete frame + data, err := hex.DecodeString("414d515000060606010000000000") + assert.Nil(t, err) + + stream := &amqpStream{data: data, message: new(amqpMessage)} + ok, complete := amqp.amqpMessageParser(stream) + + if !ok { + t.Errorf("Parsing should not raise an error") + } + if complete { + t.Errorf("message should not be complete") + } +} + func TestAmqp_WrongShortStringSize(t *testing.T) { if testing.Verbose() { logp.LogInit(logp.LOG_DEBUG, "", false, true, []string{"amqp", "amqpdetailed"})