fix(filebeat): prevent panic in dissect processor with invalid field name

changelog/fragments/1764614942-fix-panic-in-dissect-processor-with-invalid-field-name.yaml

@@ -0,0 +1,45 @@
+# REQUIRED
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# REQUIRED for all kinds
+# Change summary; a 80ish characters long description of the change.
+summary: Prevent panic during startup if dissect processor has invalid field name in tokenizer
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# description:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# impact:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# action:
+
+# REQUIRED for all kinds
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: filebeat
+
+# AUTOMATED
+# OPTIONAL to manually add other PR URLs
+# PR URL: A link the PR that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+# pr: https://github.com/owner/repo/1234
+
+# AUTOMATED
+# OPTIONAL to manually add other issue URLs
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+# issue: https://github.com/owner/repo/1234

libbeat/processors/dissect/const.go

@@ -44,7 +44,7 @@
 	numberRE = "\\d{1,2}"
 	alphaRE  = "[[:alpha:]]*"
 
 	delimiterRE = regexp.MustCompile("(?s)(.*?)%\\{([^}]*?)}")
 	suffixRE    = regexp.MustCompile("(.+?)" + // group 1 for key name
 		"(" + ordinalIndicator + "(" + numberRE + ")" + ")?" + // group 2, 3 for ordinal
 		"(" + fixedLengthIndicator + "(" + numberRE + ")" + ")?" + // group 4, 5 for fixed length
@@ -58,7 +58,8 @@
 	errEmpty                     = errors.New("empty string provided")
 	errMixedPrefixIndirectAppend = errors.New("mixed prefix `&+`")
 	errMixedPrefixAppendIndirect = errors.New("mixed prefix `&+`")
 	errEmptyKey                  = errors.New("empty key")
 	errInvalidDatatype           = errors.New("invalid data type")
 	errMissingDatatype           = errors.New("missing data type")
+	errInvalidFieldName          = errors.New("invalid field name")
 )

libbeat/processors/dissect/dissect_test.go

@@ -20,7 +20,7 @@
 import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"os"
 	"regexp"
 	"testing"
@@ -87,20 +87,25 @@
 			},
 			Fail: false,
 		},
+		{
+			Name:     "Invalid field name should fail gracefully",
+			Tok:      "%{\n}",
+			Msg:      "test message",
+			Expected: map[string]interface{}{},
+			Fail:     true,
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
 			d, err := New(test.Tok)
-			if !assert.NoError(t, err) {
-				return
-			}
-
 			if test.Fail {
-				_, err := d.DissectConvert(test.Msg)
 				assert.Error(t, err)
 				return
 			}
+			if !assert.NoError(t, err) {
+				return
+			}
 
 			r, err := d.DissectConvert(test.Msg)
 			if !assert.NoError(t, err) {
@@ -113,7 +118,7 @@
 }
 
 func TestEmptyString(t *testing.T) {
 	d, err := New("%{hello}")
 	_, err = d.Dissect("")
 	assert.Equal(t, errEmpty, err)
 }
@@ -134,12 +139,12 @@
 func init() {
 	content, err := ioutil.ReadFile("testdata/dissect_tests.json")
 	if err != nil {
 		fmt.Printf("could not read the content of 'dissect_tests', error: %s", err)
 		os.Exit(1)
 	}
 
 	if err := json.Unmarshal(content, &tests); err != nil {
 		fmt.Printf("could not parse the content of 'dissect_tests', error: %s", err)
 		os.Exit(1)
 	}
 }
@@ -215,7 +220,7 @@
 	})
 
 	b.Run("Larger regular expression", func(b *testing.B) {
 		re := regexp.MustCompile("^(\\d{2})-(\\w{3})-(\\d{4})\\s([0-9:.]+)\\s(\\w+)\\s\\[([a-zA-Z0-9-]+)\\]\\s([a-zA-Z0-9.]+)\\s(.+)")
 
 		by := `18-Apr-2018 06:53:20.411 INFO [http-nio-8080-exec-1] org.apache.coyote.http11.Http11Processor.service Error parsing HTTP request header
  Note: further occurrences of HTTP header parsing errors will be logged at DEBUG level.
@@ -237,7 +242,7 @@
 	})
 
 	b.Run("regular expression to match end of line", func(b *testing.B) {
 		re := regexp.MustCompile("MACHINE\\[(\\w+)\\]$")
 
 		by := `18-Apr-2018 06:53:20.411 INFO [http-nio-8080-exec-1] org.apache.coyote.http11.Http11Processor.service Error parsing HTTP request header
  Note: further occurrences of HTTP header parsing errors will be logged at DEBUG level.

libbeat/processors/dissect/field.go

@@ -74,7 +74,7 @@
 }
 
 func (f baseField) MarkGreedy() {
 	f.greedy = true
 }
 
 func (f baseField) Ordinal() int {
@@ -239,7 +239,10 @@
 		return newSkipField(id), nil
 	}
 
-	key, dataType, ordinal, length, greedy := extractKeyParts(rawKey)
+	key, dataType, ordinal, length, greedy, err := extractKeyParts(rawKey)
+	if err != nil {
+		return nil, err
+	}
 
 	// rawKey will have | as suffix when data type is missing
 	if strings.HasSuffix(rawKey, dataTypeIndicator) {
@@ -331,9 +334,14 @@
 	}
 }
 
-func extractKeyParts(rawKey string) (key string, dataType string, ordinal int, length int, greedy bool) {
+func extractKeyParts(rawKey string) (key string, dataType string, ordinal int, length int, greedy bool, err error) {
 	m := suffixRE.FindAllStringSubmatch(rawKey, -1)
 
+	// Check if regex matched - if not, the field name is invalid
+	if len(m) == 0 || len(m[0]) < 9 {
+		return "", "", 0, 0, false, errInvalidFieldName
+	}
+
 	if m[0][3] != "" {
 		ordinal, _ = strconv.Atoi(m[0][3])
 	}
@@ -348,5 +356,5 @@
 
 	dataType = m[0][8]
 
-	return m[0][1], dataType, ordinal, length, greedy
+	return m[0][1], dataType, ordinal, length, greedy, nil
 }