From 7412ef61b53663349f46bbfbb08a2e668a0c4016 Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Thu, 27 Oct 2016 20:24:31 -0400 Subject: [PATCH 1/2] Fix 'index out of range' bug in Packetbeat DNS protocol plugin When converting resource records into MapStrs the code can run into an index out of range exception when processing OPT psuedo resource records. The code reduces the size of a slice by creating a new, smaller slice and copies the existing data to the smaller slice. But the code did not account for the fact that the length of the slice was smaller when indexing. Fixes #2872 (cherry picked from commit cbde106b7a915ffd781a933397786473d3c1e429) --- CHANGELOG.asciidoc | 2 ++ packetbeat/protos/dns/dns.go | 13 +++++-------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 6e9f13c2ae7..2f7b1ff5279 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -35,6 +35,8 @@ https://github.com/elastic/beats/compare/v5.0.0...5.0[Check the HEAD diff] *Packetbeat* +- Fix 'index out of bounds' bug in Packetbeat DNS protocol plugin. {issue}2872[2872] + *Topbeat* *Filebeat* diff --git a/packetbeat/protos/dns/dns.go b/packetbeat/protos/dns/dns.go index 554cd4f5cec..4c9aa5ccbe8 100644 --- a/packetbeat/protos/dns/dns.go +++ b/packetbeat/protos/dns/dns.go @@ -529,26 +529,23 @@ func optToMapStr(rrOPT *mkdns.OPT) common.MapStr { return optMapStr } -// rrsToMapStr converts an array of RR's to an array of MapStr's. +// rrsToMapStr converts an slice of RR's to an slice of MapStr's. func rrsToMapStrs(records []mkdns.RR) []common.MapStr { - mapStrArray := make([]common.MapStr, len(records)) - for i, rr := range records { + mapStrSlice := make([]common.MapStr, 0, len(records)) + for _, rr := range records { rrHeader := rr.Header() mapStr := rrToMapStr(rr) if len(mapStr) == 0 { // OPT pseudo-RR returns an empty MapStr - resizeStrArray := make([]common.MapStr, len(mapStrArray)-1) - copy(resizeStrArray, mapStrArray) - mapStrArray = resizeStrArray continue } mapStr["name"] = rrHeader.Name mapStr["type"] = dnsTypeToString(rrHeader.Rrtype) mapStr["class"] = dnsClassToString(rrHeader.Class) mapStr["ttl"] = strconv.FormatInt(int64(rrHeader.Ttl), 10) - mapStrArray[i] = mapStr + mapStrSlice = append(mapStrSlice, mapStr) } - return mapStrArray + return mapStrSlice } // Convert all RDATA fields of a RR to a single string From fcf41db27b0ec5ef1ac7b335490b53f81f4a7e7a Mon Sep 17 00:00:00 2001 From: Andrew Kroh Date: Fri, 28 Oct 2016 09:40:15 -0400 Subject: [PATCH 2/2] Added a DNS test case for rrsToMapStrs with a OPT resource record (cherry picked from commit 7c2fe0933a49dafbeaa518fb551dd07deb54f978) --- packetbeat/protos/dns/dns_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packetbeat/protos/dns/dns_test.go b/packetbeat/protos/dns/dns_test.go index 039338975ad..13442241f45 100644 --- a/packetbeat/protos/dns/dns_test.go +++ b/packetbeat/protos/dns/dns_test.go @@ -16,6 +16,7 @@ import ( "github.com/elastic/beats/libbeat/common" "github.com/elastic/beats/libbeat/logp" + mkdns "github.com/miekg/dns" "github.com/stretchr/testify/assert" ) @@ -261,3 +262,27 @@ func assertAddress(t testing.TB, expected common.IpPortTuple, endpoint interface assert.Equal(t, expected.Src_ip.String(), e.Ip) assert.Equal(t, expected.Src_port, e.Port) } + +func TestRRsToMapStrsWithOPTRecord(t *testing.T) { + o := new(mkdns.OPT) + o.Hdr.Name = "." // MUST be the root zone, per definition. + o.Hdr.Rrtype = mkdns.TypeOPT + + r := new(mkdns.MX) + r.Hdr = mkdns.RR_Header{Name: "miek.nl.", Rrtype: mkdns.TypeMX, + Class: mkdns.ClassINET, Ttl: 3600} + r.Preference = 10 + r.Mx = "mx.miek.nl." + + // The OPT record is a psuedo-record so it doesn't become a real record + // in our conversion, and there will be 1 entry instead of 2. + mapStrs := rrsToMapStrs([]mkdns.RR{o, r}) + assert.Len(t, mapStrs, 1) + + mapStr := mapStrs[0] + assert.Equal(t, "IN", mapStr["class"]) + assert.Equal(t, "MX", mapStr["type"]) + assert.Equal(t, "mx.miek.nl.", mapStr["data"]) + assert.Equal(t, "miek.nl.", mapStr["name"]) + assert.EqualValues(t, 10, mapStr["preference"]) +}