From 42081b9a28c0440b308314bbc9c5f3e56cd22edd Mon Sep 17 00:00:00 2001 From: Adrian Serrano Date: Thu, 5 Apr 2018 11:50:40 +0200 Subject: [PATCH] Fix out of bounds access to slice in MongoDB parser (#6256) * Fix out of bounds access to slice in MongoDB parser Ignore MongoDB message and drop the TCP stream if a malformed query / response is received, instead of logging a panic. Closes #5188 * Update CHANGELOG --- CHANGELOG.asciidoc | 1 + packetbeat/protos/mongodb/mongodb_parser.go | 3 ++ packetbeat/protos/mongodb/mongodb_test.go | 31 ++++++++++++++++++++- 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f587fc194fc1..184c3d112d2d 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -43,6 +43,7 @@ https://github.com/elastic/beats/compare/v5.6.8...5.6[Check the HEAD diff] - 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] +- Fix out of bounds access to slice in MongoDB parser. {pull}6256[6256] *Winlogbeat* diff --git a/packetbeat/protos/mongodb/mongodb_parser.go b/packetbeat/protos/mongodb/mongodb_parser.go index 6567c250ee0c..4631d5181672 100644 --- a/packetbeat/protos/mongodb/mongodb_parser.go +++ b/packetbeat/protos/mongodb/mongodb_parser.go @@ -341,6 +341,9 @@ func (d *decoder) readDocument() (bson.M, error) { start := d.i documentLength, err := d.readInt32() d.i = start + documentLength + if len(d.in) < d.i { + return nil, errors.New("document out of bounds") + } documentMap := bson.M{} diff --git a/packetbeat/protos/mongodb/mongodb_test.go b/packetbeat/protos/mongodb/mongodb_test.go index f7f69e3ffc25..7bf201e8dafa 100644 --- a/packetbeat/protos/mongodb/mongodb_test.go +++ b/packetbeat/protos/mongodb/mongodb_test.go @@ -7,11 +7,12 @@ import ( "net" "testing" + "github.com/stretchr/testify/assert" + "github.com/elastic/beats/libbeat/common" "github.com/elastic/beats/libbeat/logp" "github.com/elastic/beats/packetbeat/protos" "github.com/elastic/beats/packetbeat/publish" - "github.com/stretchr/testify/assert" ) // Helper function returning a Mongodb module that can be used @@ -333,3 +334,31 @@ func TestMaxDocSize(t *testing.T) { assert.Equal(t, "\"1234 ...\n\"123\"\n\"12\"", res["response"]) } + +// Test for a (recovered) panic parsing document length in request/response messages +func TestDocumentLengthBoundsChecked(t *testing.T) { + if testing.Verbose() { + logp.LogInit(logp.LOG_DEBUG, "", false, true, []string{"mongodb", "mongodbdetailed"}) + } + + mongodb := mongodbModForTests() + + // request and response from tests/pcaps/mongo_one_row.pcap + reqData, err := hex.DecodeString( + // Request message with out of bounds document + "320000000a000000ffffffffd4070000" + + "00000000746573742e72667374617572" + + "616e7473000000000001000000" + + // Document length (including itself) + "06000000" + + // Document (1 byte instead of 2) + "00") + assert.Nil(t, err) + + tcptuple := testTCPTuple() + req := protos.Packet{Payload: reqData} + private := protos.ProtocolData(new(mongodbConnectionData)) + + private = mongodb.Parse(&req, tcptuple, 0, private) + assert.NotNil(t, private, "mongodb parser recovered from a panic") +}