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.

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

Fix Linux Buffer Overflow Attempts detection to correctly use regexes #5134

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kelnage
Copy link
Contributor

@kelnage kelnage commented Dec 18, 2024

Summary of the Pull Request

Fixes the Buffer Overflow Attempts rule to correctly use the regular expressions specified in the detection items. Error spotted by flabitron in the Sigma Discord.

Changelog

update: Buffer Overflow Attempts - correct detection to utilize regexes

Example Log Event

Fixed Issues

SigmaHQ Rule Creation Conventions

  • If your PR adds new rules, please consider following and applying these conventions

@github-actions github-actions bot added Rules Linux Pull request add/update linux related rules labels Dec 18, 2024
@defensivedepth
Copy link
Contributor

This modification fundamentally changes this rule - it was never a regex rule (original commit from 2017: 2e0632b)

If this PR is accepted, the status should be downgraded from stable, as it is currently part of the core rule pack.

keyword regex rule is going to incur a performance penalty.

@kelnage
Copy link
Contributor Author

kelnage commented Dec 18, 2024

I have to somewhat disagree with you there about the utilization of regexes. The obvious example is rpc.statd[\d+] - the linux logs (/var/log/messages) that I believe this rule is designed to search over will never contain \d+ - they will instead contain the (numeric) process ID instead, e.g.:

server.example.com rpc.statd[2960]: my_svc_run() - select: Bad file descriptor

As such, that part of the detection is simply broken unless it is converted to a regex. The same is likely true for the .* in 'FTP LOGIN FROM .* 0bin0sh'. Also note that the source of these rules are all pcre2 regular expressions.

As for the performance penalty, I would suggest a bigger performance penalty is deploying a rule that includes things that won't exist?

I do however agree that we should probably downgrade the status if this PR is accepted.

Copy link
Member

@nasbench nasbench left a comment

Choose a reason for hiding this comment

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

Since the rule was broken before. This fix is necessary.
By demoting the rule to experimental, we should not include it in the core release, which should be fine.

it will be an expensive rule to run for sure, because of the nature of keyword, all linux index and regex. Which is a rare combination but necessary in this specific case.

I will be contemplating to moving this to hunting down the line or probably offer some indicator on the rule performance in some way.

Open to suggestions on how we do that either via description or perhaps a new field.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Additional Data Needed Linux Pull request add/update linux related rules Rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants