Skip to content

[bugfix] rules without score not generating alerts#47

Open
ericzinnikas wants to merge 2 commits intocarbonblack:developfrom
ericzinnikas:fix-default-scoring
Open

[bugfix] rules without score not generating alerts#47
ericzinnikas wants to merge 2 commits intocarbonblack:developfrom
ericzinnikas:fix-default-scoring

Conversation

@ericzinnikas
Copy link

@ericzinnikas ericzinnikas commented Jan 17, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Tests have been added that prove the fix is effective or that the feature works.
  • New and existing tests pass locally with the changes.
  • Code follows the style guidelines of this project (PEP8, clean code).
  • Linter has passed locally and any fixes were made for failures.
  • A self-review of the code has been done.

Pull request type

Please check the type of change your PR introduces:

  • Bugfix

What is the ticket or issue number?

  • CB-41404

Pull Request Description

Per this CB Knowledge Base article and what I'm assuming is the intent of the get_high_score function here:

If there is no "score" value assigned by the rule, but a hit is determined, it will get a default score of 100

However, there is a bug in the get_high_score function. Since match.meta.get("score", 0) evaluates to 0 if "score" is unset in the rule, but "score" is set to -1 before this loop, the condition will always evaluate to True. Thus, the following line match.meta.get("score") sets score to None and the return 100 line is unreachable.

This has the effect of causing rules without a score set to not generate any alerts. It appears the bug was introduced in this change: 35afcac -- when the default score value was changed from 0 to 1, the condition was not updated along with it.

While the README indicates: "Your rules must have meta section with a score = [1-10] tag to appropriately score matching binaries", this conflicts with the guidance given in the KB article, the (assumed) intent of the get_high_score function and the fact that CB seems to score alerts from 1-100 -- thus, I have updated the README as well.

Does this introduce a breaking change?

  • Yes
  • No

How Has This Been Tested?

Buggy behavior confirmed locally with test yara rules (with and without score values). New mock test added to simulate a rule with an empty meta section and thus no set "score" value. Have not yet been able to create manual rpm package to test this fix directly on a CB server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant