From 712e35c6f74c3876ea9a4cab01db61ffc88d8b92 Mon Sep 17 00:00:00 2001 From: Willi Ballenthin Date: Wed, 29 Jan 2025 10:00:09 +0000 Subject: [PATCH 1/3] feat: add lint to validate rule dependency scope compatibility closes #2124 --- CHANGELOG.md | 1 + scripts/lint.py | 109 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8ddce6d0..0586c800c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### New Features - add span-of-calls scope to match features against a across a sliding window of API calls within a thread @williballenthin #2532 +- add lint to catch rules that depend on other rules with impossible scope @williballenthin #2124 ### Breaking Changes diff --git a/scripts/lint.py b/scripts/lint.py index 4c4e09322..4765a3919 100644 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -47,7 +47,7 @@ import capa.helpers import capa.features.insn import capa.capabilities.common -from capa.rules import Rule, RuleSet +from capa.rules import Rule, Scope, RuleSet from capa.features.common import OS_AUTO, String, Feature, Substring from capa.render.result_document import RuleMetadata @@ -74,11 +74,11 @@ class Lint: WARN = "[yellow]WARN[/yellow]" FAIL = "[red]FAIL[/red]" - name = "lint" - level = FAIL - recommendation = "" + name: str = "lint" + level: str = FAIL + recommendation: str = "" - def check_rule(self, ctx: Context, rule: Rule): + def check_rule(self, ctx: Context, rule: Rule) -> bool: return False @@ -478,6 +478,102 @@ def rec(statement): return self.violation +class RuleDependencyScopeMismatch(Lint): + name = "rule dependency scope mismatch" + level = Lint.FAIL + recommendation_template: str = "rule '{:s}' ({:s}) depends on rule '{:s}' ({:s})." + + def check_rule(self, ctx: Context, rule: Rule): + # get all rules by name for quick lookup + rules_by_name = {r.name: r for r in ctx.rules.rules.values()} + + # get all dependencies of this rule + namespaces = ctx.rules.rules_by_namespace + dependencies = rule.get_dependencies(namespaces) + + for dep_name in dependencies: + if dep_name not in rules_by_name: + # another lint will catch missing dependencies + continue + + dep_rule = rules_by_name[dep_name] + + if rule.scopes.static and not self._is_static_scope_compatible(rule, dep_rule): + self.recommendation = self.recommendation_template.format( + rule.name, + rule.scopes.static or "static: unsupported", + dep_name, + dep_rule.scopes.static or "static: unsupported", + ) + return True + + if rule.scopes.dynamic and not self._is_dynamic_scope_compatible(rule, dep_rule): + self.recommendation = self.recommendation_template.format( + rule.name, + rule.scopes.dynamic or "dynamic: unsupported", + dep_name, + dep_rule.scopes.dynamic or "dynamic: unsupported", + ) + return True + + return False + + @staticmethod + def _is_static_scope_compatible(parent: Rule, child: Rule) -> bool: + """ + A child rule's scope is compatible if it is equal to or lower than the parent scope. + """ + + if parent.scopes.static and not child.scopes.static and child.is_subscope_rule(): + # this is ok: the child isn't a static subscope rule + return True + + if parent.scopes.static and not child.scopes.static: + # This is not really ok, but we can't really be sure here: + # the parent is a static rule, and the child is not, + # and we don't know if this is strictly required to match. + # Assume for now it is not. + return True + + static_scope_order = [ + None, + Scope.FILE, + Scope.FUNCTION, + Scope.BASIC_BLOCK, + Scope.INSTRUCTION, + ] + + return static_scope_order.index(child.scopes.static) >= static_scope_order.index(parent.scopes.static) + + @staticmethod + def _is_dynamic_scope_compatible(parent: Rule, child: Rule) -> bool: + """ + A child rule's scope is compatible if it is equal to or lower than the parent scope. + """ + + if parent.scopes.dynamic and not child.scopes.dynamic and child.is_subscope_rule(): + # this is ok: the child isn't a dynamic subscope rule + return True + + if parent.scopes.dynamic and not child.scopes.dynamic: + # This is not really ok, but we can't really be sure here: + # the parent is a dynamic rule, and the child is not, + # and we don't know if this is strictly required to match. + # Assume for now it is not. + return True + + dynamic_scope_order = [ + None, + Scope.FILE, + Scope.PROCESS, + Scope.THREAD, + Scope.SPAN_OF_CALLS, + Scope.CALL, + ] + + return dynamic_scope_order.index(child.scopes.dynamic) >= dynamic_scope_order.index(parent.scopes.dynamic) + + class OptionalNotUnderAnd(Lint): name = "rule contains an `optional` or `0 or more` statement that's not found under an `and` statement" recommendation = "clarify the rule logic and ensure `optional` and `0 or more` is always found under `and`" @@ -820,6 +916,7 @@ def rec(statement): OrStatementWithAlwaysTrueChild(), NotNotUnderAnd(), OptionalNotUnderAnd(), + RuleDependencyScopeMismatch(), ) @@ -915,7 +1012,7 @@ def lint(ctx: Context): source_rules = [rule for rule in ctx.rules.rules.values() if not rule.is_subscope_rule()] n_rules: int = len(source_rules) - with capa.helpers.CapaProgressBar(transient=True, console=capa.helpers.log_console) as pbar: + with capa.helpers.CapaProgressBar(transient=True, console=capa.helpers.log_console, disable=True) as pbar: task = pbar.add_task(description="linting", total=n_rules, unit="rule") for rule in source_rules: name = rule.name From 618a5fa2e59d21aba388cbc1a9ff605029fd04d6 Mon Sep 17 00:00:00 2001 From: Willi Ballenthin Date: Wed, 29 Jan 2025 09:35:06 +0000 Subject: [PATCH 2/3] pyproject: remove pytest-cov closes #2491 --- CHANGELOG.md | 5 +++-- pyproject.toml | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0586c800c..9113f6dac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,12 +21,13 @@ ### Bug Fixes -- vmray: load more analysis archives @mr-tz - dynamic: only check file limitations for static file formats @mr-tz +- vmray: load more analysis archives @mr-tz - vmray: skip non-printable strings @mike-hunhoff +- vmray: loosen file checks to enable processing more file types @mike-hunhoff #2571 - strings: add type hints and fix uncovered bugs @williballenthin #2555 - elffile: handle symbols without a name @williballenthin #2553 -- vmray: loosen file checks to enable processing more file types @mike-hunhoff #2571 +- project: remove pytest-cov that wasn't used @williballenthin @2491 ### capa Explorer Web diff --git a/pyproject.toml b/pyproject.toml index ec61ef830..6a65f476e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,7 +125,6 @@ dev = [ "pytest==8.0.0", "pytest-sugar==1.0.0", "pytest-instafail==0.5.0", - "pytest-cov==6.0.0", "flake8==7.1.1", "flake8-bugbear==24.12.12", "flake8-encodings==0.5.1", @@ -225,7 +224,6 @@ DEP002 = [ "PyGithub", "pyinstaller", "pytest", - "pytest-cov", "pytest-instafail", "pytest-sugar", "ruff", From 91d0d8c2121d3c7aff79ccc6ece46972710e987c Mon Sep 17 00:00:00 2001 From: Capa Bot Date: Wed, 29 Jan 2025 17:55:01 +0000 Subject: [PATCH 3/3] Sync capa rules submodule --- rules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules b/rules index ff76d01d6..0ea339494 160000 --- a/rules +++ b/rules @@ -1 +1 @@ -Subproject commit ff76d01d62095839d8f430cc5538bf5c678e138b +Subproject commit 0ea3394942e17cdf44f94fff6893c98070069daa