Skip to content

Commit 10782e4

Browse files
[Filebeat] Fix panic in decode_cef when recovering from invalid data (#30038) (#30042)
* Fix panic in decode_cef when recovering from invalid data When recovering from an invalid extension value the escape sequence state was not cleared. This caused the parser to attempt to unescape the next extension which resulted in invalid data or a panic. Fixes #30010 * Encapsulate non-ragel state Document and encapsulate the non-ragel state variables. ``` $ benchcmp before.txt after.txt benchmark old ns/op new ns/op delta BenchmarkEventUnpack-12 1991 1544 -22.45% benchmark old allocs new allocs delta BenchmarkEventUnpack-12 13 13 +0.00% benchmark old bytes new bytes delta BenchmarkEventUnpack-12 642 642 +0.00% ``` (cherry picked from commit 47b8d02) Co-authored-by: Andrew Kroh <[email protected]>
1 parent a574d89 commit 10782e4

File tree

5 files changed

+219
-173
lines changed

5 files changed

+219
-173
lines changed

CHANGELOG.next.asciidoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
6060
- Make Cisco ASA and FTD modules conform to the ECS definition for event.outcome and event.type. {issue}29581[29581] {pull}29698[29698]
6161
- ibmmq: Fixed `@timestamp` not being populated with correct values. {pull}29773[29773]
6262
- aws-s3: Improve gzip detection to avoid false negatives. {issue}29968[29968]
63+
- decode_cef: Fix panic when recovering from invalid CEF extensions that contain escape characters. {issue}30010[30010]
6364

6465
*Heartbeat*
6566

x-pack/filebeat/processors/decode_cef/cef/cef.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,28 +152,31 @@ func (e *Event) Unpack(data string, opts ...Option) error {
152152
return multierr.Combine(errs...)
153153
}
154154

155+
type escapePosition struct {
156+
start, end int
157+
}
158+
155159
// replaceEscapes replaces the escaped characters contained in v with their
156160
// unescaped value.
157-
func replaceEscapes(v string, startOffset int, escapes []int) string {
161+
func replaceEscapes(v string, startOffset int, escapes []escapePosition) string {
158162
if len(escapes) == 0 {
159163
return v
160164
}
161165

162166
// Adjust escape offsets relative to the start offset of v.
163167
for i := 0; i < len(escapes); i++ {
164-
escapes[i] = escapes[i] - startOffset
168+
escapes[i].start = escapes[i].start - startOffset
169+
escapes[i].end = escapes[i].end - startOffset
165170
}
166171

167172
var buf strings.Builder
168-
var end int
173+
var prevEnd int
169174

170175
// Iterate over escapes and replace them.
171-
for i := 0; i < len(escapes); i += 2 {
172-
start := escapes[i]
173-
buf.WriteString(v[end:start])
176+
for _, escape := range escapes {
177+
buf.WriteString(v[prevEnd:escape.start])
174178

175-
end = escapes[i+1]
176-
value := v[start:end]
179+
value := v[escape.start:escape.end]
177180

178181
switch value {
179182
case `\n`:
@@ -186,8 +189,10 @@ func replaceEscapes(v string, startOffset int, escapes []int) string {
186189
buf.WriteString(value[1:])
187190
}
188191
}
192+
193+
prevEnd = escape.end
189194
}
190-
buf.WriteString(v[end:])
195+
buf.WriteString(v[prevEnd:])
191196

192197
return buf.String()
193198
}

x-pack/filebeat/processors/decode_cef/cef/cef.rl

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,31 @@ import (
1515
variable pe pe;
1616
}%%
1717

18+
type cefState struct {
19+
key string // Extension key.
20+
valueStart int // Start index of extension value.
21+
valueEnd int // End index of extension value.
22+
escapes []escapePosition // Array of escapes indices within the current value.
23+
}
24+
25+
func (s *cefState) reset() {
26+
s.key = ""
27+
s.valueStart = 0
28+
s.valueEnd = 0
29+
s.escapes = s.escapes[:0]
30+
}
31+
32+
func (s *cefState) pushEscape(start, end int) {
33+
s.escapes = append(s.escapes, escapePosition{start, end})
34+
}
35+
1836
// unpack unpacks a CEF message.
1937
func (e *Event) unpack(data string) error {
2038
cs, p, pe, eof := 0, 0, len(data), len(data)
2139
mark, mark_slash := 0, 0
22-
var escapes []int
23-
24-
// Extension key.
25-
var extKey string
2640

27-
// Extension value start and end indices.
28-
extValueStart, extValueEnd := 0, 0
41+
// state related to CEF values.
42+
var state cefState
2943

3044
// recoveredErrs are problems with the message that the parser was able to
3145
// recover from (though the parsing might not be "correct").
@@ -42,62 +56,62 @@ func (e *Event) unpack(data string) error {
4256
mark_slash = p
4357
}
4458
action mark_escape {
45-
escapes = append(escapes, mark_slash, p)
59+
state.pushEscape(mark_slash, p)
4660
}
4761
action version {
4862
e.Version, _ = strconv.Atoi(data[mark:p])
4963
}
5064
action device_vendor {
51-
e.DeviceVendor = replaceEscapes(data[mark:p], mark, escapes)
52-
escapes = escapes[:0]
65+
e.DeviceVendor = replaceEscapes(data[mark:p], mark, state.escapes)
66+
state.reset()
5367
}
5468
action device_product {
55-
e.DeviceProduct = replaceEscapes(data[mark:p], mark, escapes)
56-
escapes = escapes[:0]
69+
e.DeviceProduct = replaceEscapes(data[mark:p], mark, state.escapes)
70+
state.reset()
5771
}
5872
action device_version {
59-
e.DeviceVersion = replaceEscapes(data[mark:p], mark, escapes)
60-
escapes = escapes[:0]
73+
e.DeviceVersion = replaceEscapes(data[mark:p], mark, state.escapes)
74+
state.reset()
6175
}
6276
action device_event_class_id {
63-
e.DeviceEventClassID = replaceEscapes(data[mark:p], mark, escapes)
64-
escapes = escapes[:0]
77+
e.DeviceEventClassID = replaceEscapes(data[mark:p], mark, state.escapes)
78+
state.reset()
6579
}
6680
action name {
67-
e.Name = replaceEscapes(data[mark:p], mark, escapes)
68-
escapes = escapes[:0]
81+
e.Name = replaceEscapes(data[mark:p], mark, state.escapes)
82+
state.reset()
6983
}
7084
action severity {
7185
e.Severity = data[mark:p]
7286
}
7387
action extension_key {
7488
// A new extension key marks the end of the last extension value.
75-
if len(extKey) > 0 && extValueStart <= mark - 1 {
76-
e.pushExtension(extKey, replaceEscapes(data[extValueStart:mark-1], extValueStart, escapes))
77-
extKey, extValueStart, extValueEnd, escapes = "", 0, 0, escapes[:0]
89+
if len(state.key) > 0 && state.valueStart <= mark - 1 {
90+
e.pushExtension(state.key, replaceEscapes(data[state.valueStart:mark-1], state.valueStart, state.escapes))
91+
state.reset()
7892
}
79-
extKey = data[mark:p]
93+
state.key = data[mark:p]
8094
}
8195
action extension_value_start {
82-
extValueStart = p;
83-
extValueEnd = p
96+
state.valueStart = p;
97+
state.valueEnd = p
8498
}
8599
action extension_value_mark {
86-
extValueEnd = p+1
100+
state.valueEnd = p+1
87101
}
88102
action extension_eof {
89103
// Reaching the EOF marks the end of the final extension value.
90-
if len(extKey) > 0 && extValueStart <= extValueEnd {
91-
e.pushExtension(extKey, replaceEscapes(data[extValueStart:extValueEnd], extValueStart, escapes))
92-
extKey, extValueStart, extValueEnd, escapes = "", 0, 0, escapes[:0]
104+
if len(state.key) > 0 && state.valueStart <= state.valueEnd {
105+
e.pushExtension(state.key, replaceEscapes(data[state.valueStart:state.valueEnd], state.valueStart, state.escapes))
106+
state.reset()
93107
}
94108
}
95109
action extension_err {
96-
recoveredErrs = append(recoveredErrs, fmt.Errorf("malformed value for %s at pos %d", extKey, p+1))
110+
recoveredErrs = append(recoveredErrs, fmt.Errorf("malformed value for %s at pos %d", state.key, p+1))
97111
fhold; fnext gobble_extension;
98112
}
99113
action recover_next_extension {
100-
extKey, extValueStart, extValueEnd = "", 0, 0
114+
state.reset()
101115
// Resume processing at p, the start of the next extension key.
102116
p = mark;
103117
fnext extensions;

x-pack/filebeat/processors/decode_cef/cef/cef_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,18 @@ func TestEventUnpack(t *testing.T) {
389389
"msg": StringField("Newlines in messages\nare allowed.\r\nAnd so are carriage feeds\\newlines\\=."),
390390
}, e.Extensions)
391391
})
392+
393+
t.Run("error recovery with escape", func(t *testing.T) {
394+
// Ensure no panic or regression of https://github.com/elastic/beats/issues/30010.
395+
// key1 contains an escape, but then an invalid non-escaped =.
396+
// This triggers the error recovery to try to read the next key.
397+
var e Event
398+
err := e.Unpack(`CEF:0|||||||key1=\\hi= key2=a`)
399+
assert.Error(t, err)
400+
assert.Equal(t, map[string]*Field{
401+
"key2": UndocumentedField("a"),
402+
}, e.Extensions)
403+
})
392404
}
393405

394406
func TestEventUnpackWithFullExtensionNames(t *testing.T) {

0 commit comments

Comments
 (0)