Skip to content

Commit

Permalink
Fix 'index out of range' bug in Packetbeat DNS protocol plugin (#2875)
Browse files Browse the repository at this point in the history
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.

* Added a DNS test case for rrsToMapStrs with a OPT resource record

Fixes #2872
  • Loading branch information
andrewkroh authored and ruflin committed Oct 31, 2016
1 parent b44bdd6 commit e8b1307
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ https://github.com/elastic/beats/compare/v5.0.0...master[Check the HEAD diff]

*Packetbeat*

- Fix 'index out of bounds' bug in Packetbeat DNS protocol plugin. {issue}2872[2872]

*Topbeat*

*Filebeat*
Expand Down
13 changes: 5 additions & 8 deletions packetbeat/protos/dns/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions packetbeat/protos/dns/dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -261,3 +262,27 @@ func assertAddress(t testing.TB, expected common.IPPortTuple, endpoint interface
assert.Equal(t, expected.SrcIP.String(), e.IP)
assert.Equal(t, expected.SrcPort, 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"])
}

0 comments on commit e8b1307

Please sign in to comment.