Skip to content

Commit dd3af18

Browse files
authored
fix(filebeat): prevent panic in dissect processor with invalid field name (#47839)
When the dissect processor is instantiated with an invalid field name in the tokenizer config option, it panics. The code assumes any string is a valid field name and doesn't validate the regex results. If the field name cannot be parsed, return an error instead of panicking.
1 parent a042afd commit dd3af18

File tree

4 files changed

+67
-8
lines changed

4 files changed

+67
-8
lines changed
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# REQUIRED
2+
# Kind can be one of:
3+
# - breaking-change: a change to previously-documented behavior
4+
# - deprecation: functionality that is being removed in a later release
5+
# - bug-fix: fixes a problem in a previous version
6+
# - enhancement: extends functionality but does not break or fix existing behavior
7+
# - feature: new functionality
8+
# - known-issue: problems that we are aware of in a given version
9+
# - security: impacts on the security of a product or a user’s deployment.
10+
# - upgrade: important information for someone upgrading from a prior version
11+
# - other: does not fit into any of the other categories
12+
kind: bug-fix
13+
14+
# REQUIRED for all kinds
15+
# Change summary; a 80ish characters long description of the change.
16+
summary: Prevent panic during startup if dissect processor has invalid field name in tokenizer
17+
18+
# REQUIRED for breaking-change, deprecation, known-issue
19+
# Long description; in case the summary is not enough to describe the change
20+
# this field accommodate a description without length limits.
21+
# description:
22+
23+
# REQUIRED for breaking-change, deprecation, known-issue
24+
# impact:
25+
26+
# REQUIRED for breaking-change, deprecation, known-issue
27+
# action:
28+
29+
# REQUIRED for all kinds
30+
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
31+
component: filebeat
32+
33+
# AUTOMATED
34+
# OPTIONAL to manually add other PR URLs
35+
# PR URL: A link the PR that added the changeset.
36+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
37+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
38+
# Please provide it if you are adding a fragment for a different PR.
39+
# pr: https://github.com/owner/repo/1234
40+
41+
# AUTOMATED
42+
# OPTIONAL to manually add other issue URLs
43+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
44+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
45+
# issue: https://github.com/owner/repo/1234

libbeat/processors/dissect/const.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,5 @@ var (
6161
errEmptyKey = errors.New("empty key")
6262
errInvalidDatatype = errors.New("invalid data type")
6363
errMissingDatatype = errors.New("missing data type")
64+
errInvalidFieldName = errors.New("invalid field name")
6465
)

libbeat/processors/dissect/dissect_test.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,20 +87,25 @@ func TestDissectConversion(t *testing.T) {
8787
},
8888
Fail: false,
8989
},
90+
{
91+
Name: "Invalid field name should fail gracefully",
92+
Tok: "%{\n}",
93+
Msg: "test message",
94+
Expected: map[string]interface{}{},
95+
Fail: true,
96+
},
9097
}
9198

9299
for _, test := range tests {
93100
t.Run(test.Name, func(t *testing.T) {
94101
d, err := New(test.Tok)
95-
if !assert.NoError(t, err) {
96-
return
97-
}
98-
99102
if test.Fail {
100-
_, err := d.DissectConvert(test.Msg)
101103
assert.Error(t, err)
102104
return
103105
}
106+
if !assert.NoError(t, err) {
107+
return
108+
}
104109

105110
r, err := d.DissectConvert(test.Msg)
106111
if !assert.NoError(t, err) {

libbeat/processors/dissect/field.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,10 @@ func newField(id int, rawKey string, previous delimiter) (field, error) {
239239
return newSkipField(id), nil
240240
}
241241

242-
key, dataType, ordinal, length, greedy := extractKeyParts(rawKey)
242+
key, dataType, ordinal, length, greedy, err := extractKeyParts(rawKey)
243+
if err != nil {
244+
return nil, err
245+
}
243246

244247
// rawKey will have | as suffix when data type is missing
245248
if strings.HasSuffix(rawKey, dataTypeIndicator) {
@@ -331,9 +334,14 @@ func newNormalField(id int, key string, dataType string, ordinal int, length int
331334
}
332335
}
333336

334-
func extractKeyParts(rawKey string) (key string, dataType string, ordinal int, length int, greedy bool) {
337+
func extractKeyParts(rawKey string) (key string, dataType string, ordinal int, length int, greedy bool, err error) {
335338
m := suffixRE.FindAllStringSubmatch(rawKey, -1)
336339

340+
// check if we have at least one match otherwise the field is invalid.
341+
if len(m) == 0 {
342+
return "", "", 0, 0, false, errInvalidFieldName
343+
}
344+
337345
if m[0][3] != "" {
338346
ordinal, _ = strconv.Atoi(m[0][3])
339347
}
@@ -348,5 +356,5 @@ func extractKeyParts(rawKey string) (key string, dataType string, ordinal int, l
348356

349357
dataType = m[0][8]
350358

351-
return m[0][1], dataType, ordinal, length, greedy
359+
return m[0][1], dataType, ordinal, length, greedy, nil
352360
}

0 commit comments

Comments
 (0)