packetbeat/protos/memcache: don't panic when a transaction is empty (#…

…33853) (#33936) When a *transaction is constructed from a UDP stream, but has no request and no response, onTransaction is handed a nil *transaction, resulting in a panic in Event when the *transaction.Notes field is accessed. The panic is not recovered by ParseUDP's recover because the panic happens in a separate goroutine. So be more careful with pointer dereferencing in this path and put a separate recover in the time.AfterFunc-called closure. (cherry picked from commit 248c9b0) Co-authored-by: Dan Kortschak <[email protected]>
elastic · Dec 5, 2022 · c24f3f4 · c24f3f4
1 parent 7eac335
commit c24f3f4
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 0 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -104,6 +104,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff]
 
 *Packetbeat*
 
+- Fix panic on memcache transaction with no request or response. {issue}33852[33852] {pull}33853[33853]
 
 *Winlogbeat*
 

diff --git a/packetbeat/protos/memcache/memcache.go b/packetbeat/protos/memcache/memcache.go
@@ -198,6 +198,9 @@ func (mc *memcache) GetPorts() []int {
 }
 
 func (mc *memcache) finishTransaction(t *transaction) error {
+	if t == nil {
+		return nil
+	}
 	mc.handler.onTransaction(t)
 	return nil
 }

diff --git a/packetbeat/protos/memcache/plugin_udp.go b/packetbeat/protos/memcache/plugin_udp.go
@@ -144,6 +144,7 @@ func (mc *memcache) ParseUDP(pkt *protos.Packet) {
 	}
 	if !done {
 		trans.timer = time.AfterFunc(mc.udpConfig.transTimeout, func() {
+			defer logp.Recover("ParseMemcache(UDP) panic during forward")
 			debug("transaction timeout -> forward")
 			mc.onUDPTrans(trans)
 			mc.udpExpTrans.push(trans)
@@ -261,6 +262,9 @@ func (c *udpConnection) killTransaction(t *udpTransaction) {
 }
 
 func (lst *udpExpTransList) push(t *udpTransaction) {
+	if t == nil {
+		return
+	}
 	lst.Lock()
 	defer lst.Unlock()
 	t.next = lst.head
Original file line number	Diff line number	Diff line change
Expand Up		@@ -104,6 +104,7 @@ https://github.com/elastic/beats/compare/v8.2.0\...main[Check the HEAD diff]

		Packetbeat

		- Fix panic on memcache transaction with no request or response. {issue}33852[33852] {pull}33853[33853]

		Winlogbeat

Expand Down