diff --git a/src/openultrasast/model/config_value.py b/src/openultrasast/model/config_value.py index 1813327..3f7e50a 100644 --- a/src/openultrasast/model/config_value.py +++ b/src/openultrasast/model/config_value.py @@ -26,7 +26,9 @@ def verdict(cpg: CpgResult, spec: ConfigSpec, *, function: str = "") -> Verdict if not settings: return None + # Both abstractions read the same literal: a permissive flag ("*", True) and a weak algorithm ("md5"). permissive = {value.strip().strip("'\"").lower() for value in spec.permissive} + permissive |= {value.strip().strip("'\"").lower() for value in spec.weak_algorithms} for row in settings: literals = row["literals"] if not isinstance(literals, list): diff --git a/src/openultrasast/model/specs.py b/src/openultrasast/model/specs.py index ca0cbee..381e235 100644 --- a/src/openultrasast/model/specs.py +++ b/src/openultrasast/model/specs.py @@ -140,9 +140,15 @@ class ConfigSpec: language: str settings: tuple[str, ...] permissive: tuple[str, ...] + # CWE-326/327. A weak algorithm is a different abstraction from a permissive flag: the danger is the + # algorithm NAMED, not a setting left open. The data has been on the sink facts (`weak_literals`) since + # before this feature and nothing read it. + weak_algorithms: tuple[str, ...] = () -def config_specs(*, language: str, facts: ObligationFacts | None = None) -> Mapping[str, ConfigSpec]: +def config_specs( + *, language: str, facts: ObligationFacts | None = None, flow_facts: SemanticFacts | None = None +) -> Mapping[str, ConfigSpec]: """One ``ConfigSpec`` per configuration family, from the obligation facts' security-setting operations. ``permissive_values`` on a ``non_permissive_value`` discharger is the closed set of literals that leave a @@ -151,9 +157,23 @@ def config_specs(*, language: str, facts: ObligationFacts | None = None) -> Mapp scoped = (facts if facts is not None else load_obligation_facts()).for_language(language) settings = tuple(sorted({call for fact in scoped.operations if fact.kind == "security_setting" for call in fact.calls})) permissive = tuple(sorted({value for fact in scoped.dischargers for value in fact.permissive_values})) + # A sink carrying `weak_literals` names an algorithm choice, and the call that takes it is itself a + # security setting: `hashlib.new("md5")` is dangerous because of the string, exactly as CORS is because + # of the "*". Reading them here is what wires the second of config_secrets' three abstractions. + flow = (flow_facts if flow_facts is not None else load_facts()).for_language(language) + weak = tuple(sorted({literal for sink in flow.sinks for literal in sink.weak_literals})) + weak_calls = tuple(sorted({call for sink in flow.sinks if sink.weak_literals for call in sink.calls})) if not settings or not permissive: return {} - return {"config_secrets": ConfigSpec(family="config_secrets", language=language, settings=settings, permissive=permissive)} + return { + "config_secrets": ConfigSpec( + family="config_secrets", + language=language, + settings=tuple(sorted(set(settings) | set(weak_calls))), + permissive=permissive, + weak_algorithms=weak, + ) + } def taint_specs( diff --git a/src/openultrasast/ruleset/obligations/python.toml b/src/openultrasast/ruleset/obligations/python.toml index 84a54fc..3bf60b2 100644 --- a/src/openultrasast/ruleset/obligations/python.toml +++ b/src/openultrasast/ruleset/obligations/python.toml @@ -33,7 +33,13 @@ sensitivity = "high" [[operation]] id = "security-setting" kind = "security_setting" -calls = ["CORS", "cross_origin", "set_cookie", "app.run", "config.update", "TrustedHostMiddleware", "add_middleware", "set_debug"] +# Flask/FastAPI (CORS, set_cookie, app.run), aiohttp (session_setup, the cookie storages) and Django +# (the SECURE_* / DEBUG settings). The table was Flask-shaped while the corpus contains aiohttp and Django +# apps, so three config pairs had no modelled setting in them at all. +calls = ["CORS", "cross_origin", "set_cookie", "app.run", "config.update", "TrustedHostMiddleware", "add_middleware", "set_debug", + "session_setup", "EncryptedCookieStorage", "SimpleCookieStorage", "cookie_storage", "setup_session", + "SECURE_SSL_REDIRECT", "SESSION_COOKIE_SECURE", "CSRF_COOKIE_SECURE", "SESSION_COOKIE_HTTPONLY", "ALLOWED_HOSTS", "DEBUG", + "set_secure_cookie", "make_secure_cookie"] resource_arg = 0 requires = ["non_permissive_value"] sensitivity = "medium" @@ -59,7 +65,8 @@ calls = ["check_owner", "is_owner", "assert_owner", "ensure_owner", "owns", "can [[discharger]] id = "safe-setting" kind = "non_permissive_value" -calls = ["CORS", "cross_origin", "set_cookie", "app.run", "config.update", "add_middleware"] +calls = ["CORS", "cross_origin", "set_cookie", "app.run", "config.update", "add_middleware", + "session_setup", "SimpleCookieStorage", "setup_session", "set_secure_cookie"] permissive_values = ["'*'", "\"*\"", "True", "0.0.0.0", "None", "False", "allow_all", "disable"] [[discharger]] diff --git a/tests/test_model_config_value.py b/tests/test_model_config_value.py index 84544d5..f1e1f3a 100644 --- a/tests/test_model_config_value.py +++ b/tests/test_model_config_value.py @@ -76,3 +76,44 @@ def test_the_verdict_is_deterministic() -> None: "literalArgs": ["True", "0.0.0.0"], "args": ["x"]}] cpg = _cpg(rows) assert verdict(cpg, _spec(), function="login") == verdict(cpg, _spec(), function="login") + + +# --- framework coverage and the weak-algorithm arm -------------------------------------------------------- +# +# The settings table was Flask/FastAPI-shaped (CORS, set_cookie, app.run) while the corpus contains aiohttp +# and Django. Separately, config_secrets spans three abstractions and only one was wired: a permissive +# setting VALUE. Weak algorithm CHOICE (CWE-326/327) has its data already -- `weak_literals` on the hashlib +# sink fact -- and nothing read it. + + +def test_the_settings_table_covers_the_frameworks_in_the_corpus() -> None: + from openultrasast.model.specs import config_specs + + settings = set(config_specs(language="python")["config_secrets"].settings) + for framework, call in (("aiohttp", "session_setup"), ("aiohttp", "EncryptedCookieStorage"), + ("django", "SECURE_SSL_REDIRECT"), ("flask", "set_cookie")): + assert call in settings, f"{framework}'s {call} is not a modelled security setting" + + +def test_a_weak_algorithm_is_entailed_by_its_literal() -> None: + """CWE-327: the danger is the algorithm NAMED, not a permissive flag.""" + from openultrasast.model.config_value import verdict + from openultrasast.model.ladder import Rung + from openultrasast.model.specs import config_specs + + spec = config_specs(language="python")["config_secrets"] + rows = [{"setting": 'hashlib.new("md5")', "line": "4", "method": "digest", + "literalArgs": ['"md5"'], "args": ['"md5"']}] + answer = verdict(_cpg(rows), spec, function="digest") + assert answer is not None and answer.rung is Rung.ENTAILED + assert "md5" in answer.witness.lower() + + +def test_a_strong_algorithm_yields_no_finding() -> None: + from openultrasast.model.config_value import verdict + from openultrasast.model.specs import config_specs + + spec = config_specs(language="python")["config_secrets"] + rows = [{"setting": 'hashlib.new("sha256")', "line": "4", "method": "digest", + "literalArgs": ['"sha256"'], "args": ['"sha256"']}] + assert verdict(_cpg(rows), spec, function="digest") is None diff --git a/tests/test_model_pipeline.py b/tests/test_model_pipeline.py index 8d851e2..6b6a480 100644 --- a/tests/test_model_pipeline.py +++ b/tests/test_model_pipeline.py @@ -176,8 +176,6 @@ def test_the_residual_question_matches_the_arbiter_that_raised_it() -> None: """ from openultrasast.model.pipeline import residual_question - rows = [{"operation": "q.filter_by(id=n)", "opLine": "9", "opMethod": "leaky", "dominatingGuards": []}, - {"operation": "q.filter_by(id=n)", "opLine": "3", "opMethod": "safe", "dominatingGuards": ["current_user"]}] dom = residual_question(_dominance_spec(), "leaky line 9: no discharging guard", "leaky") assert "sanitizer" not in dom.lower(), "an absence bug has no sanitizer to judge" assert "guard" in dom.lower()