[Filebeat] Fix panic in decode_cef when recovering from invalid data #30038

andrewkroh · 2022-01-26T21:15:26Z

What does this PR do?

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

Why is it important?

The parser could panic.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

fuzz the parser

Related issues

Fixes CEF: Panic handling datagram. Recovering, but please report this. #30010

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 elastic#30010

botelastic · 2022-01-26T21:15:35Z

This pull request doesn't have a Team:<team> label.

efd6 · 2022-01-26T21:23:44Z

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

@@ -97,7 +97,7 @@ func (e *Event) unpack(data string) error {
            fhold; fnext gobble_extension;
        }
        action recover_next_extension {
-            extKey, extValueStart, extValueEnd = "", 0, 0
+            extKey, extValueStart, extValueEnd, escapes = "", 0, 0, escapes[:0]


It would be nice to have these non-ragel state variables have some commentary in the decl above.

I did a little bit of refactoring to make things slightly more readable. Will you look again please?

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% ```

efd6

Nice.

Not for here, but there are a bunch of actions that have unnecessary semis in Go statements.

elasticmachine · 2022-01-26T22:51:23Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-01-26T22:09:43.672+0000
Duration: 97 min 1 sec
Commit: faa41b6

Test stats 🧪

Test	Results
Failed	0
Passed	2427
Skipped	153
Total	2580

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

efd6 · 2022-01-26T23:11:17Z

The failures here are the same I see at the npcap PR. #30007 may be related?

…30038) * 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)

…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 <andrew.kroh@elastic.co>

…30038) (#30043) * 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 <andrew.kroh@elastic.co>

…lastic#30038) * 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 elastic#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% ```

…k-version-after-8-0-creation * upstream/master: (69 commits) Update stale config following (elastic#30082) Make include_matches backwards compatible with 7.x config (elastic#30032) [Filebeat] Update handling of elasticsearch server logs (elastic#30018) Remove SSL3 support from libbeat and its documentation. (elastic#30071) Revert "Packaging: rename arm64 suffix to aarch64 in the tar.gz artifacts ONLY (elastic#28813)" (elastic#30083) [libbeat] Add script processor to all beats (elastic#29752) Add fonts to support more different types of characters for multiple languages (elastic#29861) libbeat/reader: Fix messge conversion to beat.Event (elastic#30057) probot[stale]: ignore issues with the tag flaky-test (elastic#30065) [DOCS] Add redirect for GSuite module (elastic#30034) [Automation] Update elastic stack version to 8.1.0-aa69d697 for testing (elastic#30012) Remove msitools install for windows build, using the latest docker image with msitools preinstalled (elastic#30040) filebeat/generator/fields: fix dropped error (elastic#29943) Include the error message with auditd module events (elastic#30009) [Metricbeat] gcp: add firestore metricset (elastic#29918) probot: update stale dates (elastic#29997) Metricbeat enterprise search module: add xpack.enabled support (elastic#29871) x-pack/packetbeat: install Npcap at start-up when required (elastic#29112) [Filebeat] Fix panic in decode_cef when recovering from invalid data (elastic#30038) Correctly fixe how selected packages are defined (elastic#30039) ...

Fix panic in decode_cef when recovering from invalid data

5803c76

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 elastic#30010

andrewkroh added bug Filebeat Filebeat backport-v8.0.0 Automated backport with mergify backport-7.17 Automated backport to the 7.17 branch with mergify labels Jan 26, 2022

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 26, 2022

andrewkroh mentioned this pull request Jan 26, 2022

CEF: Panic handling datagram. Recovering, but please report this. #30010

Closed

andrewkroh requested a review from a team January 26, 2022 21:17

efd6 approved these changes Jan 26, 2022

View reviewed changes

andrewkroh requested a review from efd6 January 26, 2022 22:10

efd6 approved these changes Jan 26, 2022

View reviewed changes

andrewkroh merged commit 47b8d02 into elastic:master Jan 27, 2022

mergify bot mentioned this pull request Jan 27, 2022

[7.17](backport #30038) [Filebeat] Fix panic in decode_cef when recovering from invalid data #30042

Merged

mergify bot mentioned this pull request Jan 27, 2022

[8.0](backport #30038) [Filebeat] Fix panic in decode_cef when recovering from invalid data #30043

Merged

andrewkroh mentioned this pull request Apr 23, 2022

UDP Panic Error [CEF Input Plugin] [bug] #31340

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Filebeat] Fix panic in decode_cef when recovering from invalid data #30038

[Filebeat] Fix panic in decode_cef when recovering from invalid data #30038

andrewkroh commented Jan 26, 2022 •

edited

Loading

botelastic bot commented Jan 26, 2022

efd6 Jan 26, 2022

andrewkroh Jan 26, 2022

efd6 left a comment

elasticmachine commented Jan 26, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

efd6 commented Jan 26, 2022

[Filebeat] Fix panic in decode_cef when recovering from invalid data #30038

[Filebeat] Fix panic in decode_cef when recovering from invalid data #30038

Conversation

andrewkroh commented Jan 26, 2022 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

Related issues

botelastic bot commented Jan 26, 2022

efd6 Jan 26, 2022

Choose a reason for hiding this comment

andrewkroh Jan 26, 2022

Choose a reason for hiding this comment

efd6 left a comment

Choose a reason for hiding this comment

elasticmachine commented Jan 26, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

efd6 commented Jan 26, 2022

andrewkroh commented Jan 26, 2022 •

edited

Loading

elasticmachine commented Jan 26, 2022 •

edited by jenkins-beats-ci bot

Loading