Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Winlogbeat] protect against accessing undefined variable in security module #22937

Merged
merged 2 commits into from
Dec 7, 2020
Merged

[Winlogbeat] protect against accessing undefined variable in security module #22937

merged 2 commits into from
Dec 7, 2020

Conversation

Pearson-k
Copy link
Contributor

What does this PR do?

This pull request protects against trying to use string functions against a variable which is undefined. This has already been done for two other variables, but not this one:

logonSuccess

https://github.com/elastic/beats/blob/8369eff1c4d75fd164a8b171f1f2481c1a13932b/x-pack/winlogbeat/module/security/config/winlogbeat-security.js#L1689-L1704

event4648

https://github.com/elastic/beats/blob/8369eff1c4d75fd164a8b171f1f2481c1a13932b/x-pack/winlogbeat/module/security/config/winlogbeat-security.js#L1707-L1721

Why is it important?

In some environments, as much as 99% of some events are showing this error (particularly with event id 4625)

TypeError: Cannot read property 'split' of undefined at c:\Program Files\winlogbeat-7.10.0/module/security/config/winlogbeat-security.js:1522:24(16)

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

  • [ ]

How to test this PR locally

go test

Related issues

Similar work was recently done for the Sysmon module: #22236 #22219

Use cases

Screenshots

Logs

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Dec 4, 2020
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Dec 4, 2020
@Pearson-k Pearson-k changed the title Adding protection for undefined variable in copy target user [Winlogbeat] protect against accessing undefined variable in security module Dec 4, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 4, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #22937 updated

  • Start Time: 2020-12-04T18:42:21.492+0000

  • Duration: 22 min 26 sec

Test stats 🧪

Test Results
Failed 0
Passed 92
Skipped 0
Total 92

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 92
Skipped 0
Total 92

@Pearson-k Pearson-k marked this pull request as ready for review December 7, 2020 16:40
@Pearson-k Pearson-k requested a review from a team as a code owner December 7, 2020 16:40
@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

andrewkroh
andrewkroh approved these changes Dec 7, 2020
View reviewed changes
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks

@andrewkroh andrewkroh added the needs_backport PR is waiting to be backported to other branches. label Dec 7, 2020
@andrewkroh andrewkroh merged commit 7c64f53 into master Dec 7, 2020
andrewkroh pushed a commit to andrewkroh/beats that referenced this pull request Dec 7, 2020
@andrewkroh
[Winlogbeat] protect against accessing undefined variable in security…
c4668a5 
… module (elastic#22937)

Adding protection for undefined variable in copy target user

(cherry picked from commit 7c64f53)
@andrewkroh andrewkroh mentioned this pull request Dec 7, 2020
Merged
2 tasks
@andrewkroh andrewkroh added v7.11.0 and removed needs_backport PR is waiting to be backported to other branches. labels Dec 7, 2020
andrewkroh added a commit that referenced this pull request Dec 9, 2020
@andrewkroh
Cherry-pick #22937 to 7.x: [Winlogbeat] protect against accessing und…
b6c5f86 
…efined variable in security module (#22971)

* [Winlogbeat] protect against accessing undefined variable in security module (#22937)

Adding protection for undefined variable in copy target user

(cherry picked from commit 7c64f53)

Co-authored-by: Kyle Pearson <kyle.pearson@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewkroh andrewkroh andrewkroh approved these changes

Assignees
No one assigned
Labels
v7.11.0 Winlogbeat
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Pearson-k @elasticmachine @andrewkroh