From cf8e96e6b6357893e892d4e0857e0b72fc9455e3 Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Wed, 14 Feb 2018 12:23:04 +0100 Subject: [PATCH] AMQP: Fix panic during parse of partial message 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 | 22 ++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 1009a7e0e73..ede4b726041 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -139,6 +139,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di - 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 11ff24e866a..7141f495b50 100644 --- a/packetbeat/protos/amqp/amqp_parser.go +++ b/packetbeat/protos/amqp/amqp_parser.go @@ -72,7 +72,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 0299ee22334..e435958d15e 100644 --- a/packetbeat/protos/amqp/amqp_test.go +++ b/packetbeat/protos/amqp/amqp_test.go @@ -89,6 +89,28 @@ 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) { + logp.TestingSetup(logp.WithSelectors("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) { logp.TestingSetup(logp.WithSelectors("amqp", "amqpdetailed"))