From decacb5dbdede2fded699d72b244e817ed07e061 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Fri, 3 Apr 2026 15:53:52 -0400 Subject: [PATCH] systemd+dbus: 5-MOAD scan; systemd-0004 CWE-407 seccomp strv O(K*A), dbus-0001 CWE-407 policy optimize O(R^2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit systemd-0004: seccomp_load_syscall_filter_set() in src/shared/seccomp-util.c uses strv_contains(added, name) — O(|added|) linear scan — inside NULSTR_FOREACH over ~537 KNOWN syscalls. On x86_64 (3 arches): ~484,000 string comparisons per service start with SeccompFilter=. Sibling function seccomp_load_syscall_filter_set_raw() already uses hashmap_contains for O(1); this function was left behind. Fix: build Set* from added strv before the NULSTR_FOREACH loop. MEDIUM severity. dbus-0001: bus_client_policy_optimize() in bus/policy.c iterates R rules and for each blanket deny/allow calls remove_rules_by_type_up_to() which scans backward from current position to head — O(R^2) total per new connection creation. At R=100 rules (realistic system bus): ~10,000 comparisons per connect. Fix: single O(R) reverse pass tracking last-seen blanket per rule type. MEDIUM. dbus 5-MOAD summary: MOAD-0001: dbus-0001 DEFECT (policy optimize O(R^2)) MOAD-0002: CLEAN (BusContext is standard daemon context, not a god object) MOAD-0003: CLEAN (single-threaded event loop, no thread-local state) MOAD-0004: CLEAN (_dbus_verbose is no-op in production builds) MOAD-0005: CLEAN (pending_activations hash table coalesces duplicate requests) --- defects/dbus-0001/TICKET.md | 72 ++++++++ .../dbus-0001-policy-optimize-o-n2.patch | 110 +++++++++++ .../test/TestDbusClientPolicyOptimize.java | 173 ++++++++++++++++++ defects/dbus/patch/SCAN.md | 55 ++++++ defects/systemd-0004/TICKET.md | 72 ++++++++ defects/systemd-0004/patch/SCAN.md | 38 ++++ .../systemd-0004-seccomp-strv-to-set.patch | 44 +++++ .../test/TestSeccompStrvToSet.java | 117 ++++++++++++ 8 files changed, 681 insertions(+) create mode 100644 defects/dbus-0001/TICKET.md create mode 100644 defects/dbus-0001/patch/dbus-0001-policy-optimize-o-n2.patch create mode 100644 defects/dbus-0001/test/TestDbusClientPolicyOptimize.java create mode 100644 defects/dbus/patch/SCAN.md create mode 100644 defects/systemd-0004/TICKET.md create mode 100644 defects/systemd-0004/patch/SCAN.md create mode 100644 defects/systemd-0004/patch/systemd-0004-seccomp-strv-to-set.patch create mode 100644 defects/systemd-0004/test/TestSeccompStrvToSet.java diff --git a/defects/dbus-0001/TICKET.md b/defects/dbus-0001/TICKET.md new file mode 100644 index 000000000..2280cea2f --- /dev/null +++ b/defects/dbus-0001/TICKET.md @@ -0,0 +1,72 @@ +# dbus-0001: bus_client_policy_optimize O(R^2) rule cleanup per connection + +**MOAD:** MOAD-0001 (CWE-407) +**Severity:** MEDIUM +**Speedup:** ~50x at R=100 rules (per new connection creation) +**Language:** C +**File:** bus/policy.c:783-858 (bus_client_policy_optimize) + +## Description + +Every time a new client connects to dbus-daemon, `bus_policy_create_client_policy()` +builds a per-connection rule list from default + UID + GID + mandatory policy +rules, then calls `bus_client_policy_optimize()` to remove rules that are +guaranteed to never fire because a later blanket deny/allow supersedes them. + +The optimization iterates over all R rules and, for each blanket rule, calls +`remove_rules_by_type_up_to()` which walks backward from the current position +to the start of the list, removing earlier rules of the same type: + +```c +void bus_client_policy_optimize(BusClientPolicy *policy) { + DBusList *link = _dbus_list_get_first_link(&policy->rules); + while (link != NULL) { /* O(R) outer */ + BusPolicyRule *rule = link->data; + ... + if (remove_preceding) + remove_rules_by_type_up_to(policy, /* O(R) inner */ + rule->type, link); + link = next; + } +} +``` + +`remove_rules_by_type_up_to()` scans from the head of the list to `link`, +comparing each rule type. In the worst case (many blanket rules), this is +O(R^2) total per connection. + +A system bus with 100 default+uid+gid rules produces ~10,000 comparisons per +new connection. A session bus serving a desktop environment with many services +(GNOME, KDE) accumulates significant overhead as applications connect and +reconnect. + +The code itself notes this as a known issue — the FIXME comment in +`bus_matchmaker_disconnected()` acknowledges similar O(N) scan issues. + +## Fix + +The optimize pass can be done in a single O(R) pass by processing rules in +reverse order (from tail to head). When a blanket rule is encountered, mark +all preceding same-type rules as removable in one forward scan, then do one +cleanup pass. Alternatively, group rules by type during construction and apply +"last blanket wins" logic without backward scanning. + +A simpler fix: instead of walking backward to remove preceding rules, iterate +from the tail of the list forward. Since "last rule wins", the first blanket +encountered from the tail supersedes everything before it of the same type — +so rules before the blanket can be dropped in O(1) by truncating the list up +to that point. + +Complexity drops from O(R^2) to O(R) per connection. + +## Speedup estimate + +At R=100 rules (realistic system bus configuration): +- Defect: ~5,000 comparisons worst case +- Fix: ~100 comparisons +- Speedup: ~50x + +At R=300 rules (large desktop session bus with many UID-specific rules): +- Defect: ~45,000 comparisons +- Fix: ~300 comparisons +- Speedup: ~150x diff --git a/defects/dbus-0001/patch/dbus-0001-policy-optimize-o-n2.patch b/defects/dbus-0001/patch/dbus-0001-policy-optimize-o-n2.patch new file mode 100644 index 000000000..bbdfcdd62 --- /dev/null +++ b/defects/dbus-0001/patch/dbus-0001-policy-optimize-o-n2.patch @@ -0,0 +1,110 @@ +# UNDF: UNDF-PENDING +--- a/bus/policy.c ++++ b/bus/policy.c +@@ -780,38 +780,58 @@ remove_rules_by_type_up_to (BusClientPolicy *policy, + void + bus_client_policy_optimize (BusClientPolicy *policy) + { +- DBusList *link; ++ /* Replace O(R^2) backward-scan optimization with a single O(R) reverse pass. ++ * ++ * Original approach: for each blanket rule, call remove_rules_by_type_up_to() ++ * which walks from head to the current position — O(R) inner loop, O(R^2) total. ++ * ++ * Fixed approach: iterate from tail to head. The first (rightmost) blanket rule ++ * of each type supersedes all earlier rules of the same type. Track the last ++ * seen blanket per type and remove all preceding same-type rules in one pass. ++ */ ++ DBusList *link; ++ dbus_bool_t seen_send_blanket = FALSE; ++ dbus_bool_t seen_receive_blanket = FALSE; ++ dbus_bool_t seen_own_blanket = FALSE; + + _dbus_verbose ("Optimizing policy with %d rules\n", + _dbus_list_get_length (&policy->rules)); +- +- link = _dbus_list_get_first_link (&policy->rules); ++ ++ /* Walk from tail to head. Remove any rule that is shadowed by a later ++ * blanket rule of the same type. */ ++ link = _dbus_list_get_last_link (&policy->rules); + while (link != NULL) + { + BusPolicyRule *rule; +- DBusList *next; +- dbus_bool_t remove_preceding; ++ DBusList *prev; ++ dbus_bool_t is_blanket; ++ dbus_bool_t already_shadowed; + +- next = _dbus_list_get_next_link (&policy->rules, link); ++ prev = _dbus_list_get_prev_link (&policy->rules, link); + rule = link->data; +- +- remove_preceding = FALSE; + +- _dbus_assert (rule != NULL); +- ++ is_blanket = FALSE; ++ already_shadowed = FALSE; ++ + switch (rule->type) + { + case BUS_POLICY_RULE_SEND: +- remove_preceding = +- rule->d.send.message_type == DBUS_MESSAGE_TYPE_INVALID && ++ is_blanket = (rule->d.send.message_type == DBUS_MESSAGE_TYPE_INVALID && + rule->d.send.path == NULL && + rule->d.send.interface == NULL && + rule->d.send.member == NULL && + rule->d.send.error == NULL && +- rule->d.send.destination == NULL; ++ rule->d.send.destination == NULL); ++ already_shadowed = seen_send_blanket && !is_blanket; ++ if (is_blanket) seen_send_blanket = TRUE; + break; + case BUS_POLICY_RULE_RECEIVE: +- remove_preceding = +- rule->d.receive.message_type == DBUS_MESSAGE_TYPE_INVALID && ++ is_blanket = (rule->d.receive.message_type == DBUS_MESSAGE_TYPE_INVALID && + rule->d.receive.path == NULL && + rule->d.receive.interface == NULL && + rule->d.receive.member == NULL && + rule->d.receive.error == NULL && +- rule->d.receive.origin == NULL; ++ rule->d.receive.origin == NULL); ++ already_shadowed = seen_receive_blanket && !is_blanket; ++ if (is_blanket) seen_receive_blanket = TRUE; + break; + case BUS_POLICY_RULE_OWN: +- remove_preceding = +- rule->d.own.service_name == NULL; ++ is_blanket = (rule->d.own.service_name == NULL); ++ already_shadowed = seen_own_blanket && !is_blanket; ++ if (is_blanket) seen_own_blanket = TRUE; + break; + +- /* The other rule types don't appear in this list */ + case BUS_POLICY_RULE_USER: + case BUS_POLICY_RULE_GROUP: + default: + _dbus_assert_not_reached ("invalid rule"); + break; + } + +- if (remove_preceding) +- remove_rules_by_type_up_to (policy, rule->type, +- link); +- +- link = next; ++ if (already_shadowed) ++ { ++ /* This specific rule can never fire — remove it in O(1). */ ++ _dbus_list_remove_link (&policy->rules, link); ++ bus_policy_rule_unref (rule); ++ } ++ ++ link = prev; + } + + _dbus_verbose ("After optimization, policy has %d rules\n", diff --git a/defects/dbus-0001/test/TestDbusClientPolicyOptimize.java b/defects/dbus-0001/test/TestDbusClientPolicyOptimize.java new file mode 100644 index 000000000..2b9a608b5 --- /dev/null +++ b/defects/dbus-0001/test/TestDbusClientPolicyOptimize.java @@ -0,0 +1,173 @@ +package defects.dbus_0001; + +import org.junit.Test; +import static org.junit.Assert.*; + +import java.util.*; + +/** + * Unit test demonstrating dbus-0001 CWE-407: + * bus_client_policy_optimize() in bus/policy.c uses O(R^2) nested list scan + * to remove shadowed policy rules during connection creation. + * + * For each blanket-deny/allow rule, remove_rules_by_type_up_to() walks + * backward through the rule list, producing O(R^2) total operations. + * + * Fix: single O(R) reverse pass, tracking last blanket per type. + */ +public class TestDbusClientPolicyOptimize { + + enum RuleType { SEND, RECEIVE, OWN } + + static class PolicyRule { + RuleType type; + boolean isBlanket; // true = no field constraints = blanket deny/allow + + PolicyRule(RuleType type, boolean isBlanket) { + this.type = type; + this.isBlanket = isBlanket; + } + } + + /** + * Simulates defect: O(R^2) nested scan. + * Returns total comparisons performed. + */ + static long defect_optimize(LinkedList rules) { + long comparisons = 0; + ListIterator it = rules.listIterator(); + while (it.hasNext()) { + PolicyRule rule = it.next(); + int pos = it.previousIndex(); + if (rule.isBlanket) { + // remove_rules_by_type_up_to: walk from head to current pos + ListIterator back = rules.listIterator(); + int idx = 0; + while (back.hasNext() && idx < pos) { + PolicyRule r = back.next(); + comparisons++; // type comparison + if (r.type == rule.type) { + back.remove(); + it = rules.listIterator(pos); // reset iterator after removal + pos--; + } + idx++; + } + } + } + return comparisons; + } + + /** + * Simulates fix: O(R) reverse pass. + * Returns total comparisons performed. + */ + static long fix_optimize(LinkedList rules) { + long comparisons = 0; + // seen_blanket per type + boolean[] seenBlanket = new boolean[RuleType.values().length]; + + ListIterator it = rules.listIterator(rules.size()); + while (it.hasPrevious()) { + PolicyRule rule = it.previous(); + comparisons++; // one type check per rule + + int typeIdx = rule.type.ordinal(); + if (rule.isBlanket) { + seenBlanket[typeIdx] = true; + } else if (seenBlanket[typeIdx]) { + // This rule is shadowed by a later blanket — remove O(1) + it.remove(); + } + } + return comparisons; + } + + /** + * Build a realistic policy rule list: interleaved specific rules and + * blanket denies of each type. + */ + static LinkedList buildRules(int rulesPerType, int blanketsPerType) { + LinkedList rules = new LinkedList<>(); + for (RuleType type : RuleType.values()) { + for (int i = 0; i < rulesPerType; i++) { + rules.add(new PolicyRule(type, false)); // specific rule + if (i % (rulesPerType / Math.max(blanketsPerType, 1)) == 0) { + rules.add(new PolicyRule(type, true)); // blanket rule + } + } + } + return rules; + } + + @Test + public void testRealisticSystemBus() { + // ~100 rules across 3 types, with ~5 blanket rules each + LinkedList defect_rules = buildRules(30, 5); + LinkedList fix_rules = new LinkedList<>(defect_rules); + + long defect_ops = defect_optimize(defect_rules); + long fix_ops = fix_optimize(fix_rules); + + System.out.printf("Realistic (R~%d): defect=%,d ops, fix=%,d ops, speedup=%.1fx%n", + defect_rules.size() + fix_rules.size(), + defect_ops, fix_ops, (double) defect_ops / fix_ops); + + assertTrue("Fix should be faster than defect", fix_ops <= defect_ops); + assertTrue("Fix should have fewer ops at realistic scale", fix_ops * 5 < defect_ops); + } + + @Test + public void testLargeDesktopSessionBus() { + // ~300 rules (GNOME/KDE session bus with many UID-specific rules) + LinkedList defect_rules = buildRules(90, 10); + LinkedList fix_rules = new LinkedList<>(defect_rules); + + long defect_ops = defect_optimize(defect_rules); + long fix_ops = fix_optimize(fix_rules); + + double speedup = (double) defect_ops / fix_ops; + System.out.printf("Large desktop (R~%d): defect=%,d ops, fix=%,d ops, speedup=%.1fx%n", + defect_rules.size(), + defect_ops, fix_ops, speedup); + + assertTrue("Speedup should be > 10x at R=300", speedup > 10.0); + } + + @Test + public void testWorstCaseAllBlankets() { + // Worst case: alternating specific and blanket rules of same type + LinkedList defect_rules = new LinkedList<>(); + LinkedList fix_rules = new LinkedList<>(); + for (int i = 0; i < 50; i++) { + PolicyRule specific = new PolicyRule(RuleType.SEND, false); + PolicyRule blanket = new PolicyRule(RuleType.SEND, true); + defect_rules.add(specific); defect_rules.add(blanket); + fix_rules.add(specific); fix_rules.add(blanket); + } + + long defect_ops = defect_optimize(defect_rules); + long fix_ops = fix_optimize(fix_rules); + + double speedup = (double) defect_ops / Math.max(fix_ops, 1); + System.out.printf("Worst case (R=100, all-blankets): defect=%,d ops, fix=%,d ops, speedup=%.1fx%n", + 100, defect_ops, fix_ops, speedup); + + assertTrue("Defect must be more expensive in worst case", defect_ops > fix_ops); + } + + @Test + public void testSingleConnection() { + // Minimum case: even 1 blanket + 1 specific triggers the defect + LinkedList rules = new LinkedList<>(); + rules.add(new PolicyRule(RuleType.SEND, false)); + rules.add(new PolicyRule(RuleType.SEND, true)); + + LinkedList fix_rules = new LinkedList<>(rules); + long defect_ops = defect_optimize(rules); + long fix_ops = fix_optimize(fix_rules); + + // Defect does 1 comparison; fix does 2 checks (same for tiny case) + assertTrue("Both paths complete without error", defect_ops >= 0 && fix_ops >= 0); + } +} diff --git a/defects/dbus/patch/SCAN.md b/defects/dbus/patch/SCAN.md new file mode 100644 index 000000000..d56e6c304 --- /dev/null +++ b/defects/dbus/patch/SCAN.md @@ -0,0 +1,55 @@ +# dbus 5-MOAD scan — 2026-04-03 + +Source: https://gitlab.freedesktop.org/dbus/dbus (depth=1) +Focus: bus/ (message dispatch, activation, policy), dbus/ (auth, protocol) + +## MOAD-0001 (CWE-407): DEFECT — dbus-0001 + +`bus/policy.c` `bus_client_policy_optimize()` runs on every new connection to +eliminate shadowed policy rules. For each blanket-deny/allow rule (one with no +field constraints), it calls `remove_rules_by_type_up_to()` which walks the +full rules list from the beginning up to the current position. Total complexity +is O(R^2) where R = total rules in the client policy. + +A system bus configuration with 100 rules per connection produces ~10,000 +comparisons per new connection. Under a workload that creates and destroys many +connections (container start/stop, session bus churn), this accumulates. + +See `dbus-0001-policy-optimize-o-n2.patch` and `dbus-0001-TICKET.md` for full +analysis. + +Other CWE-407 candidates reviewed: +- `bus/signals.c` match rule dispatch: uses hash table indexed by + (message_type, interface) — pre-filtered, O(1) dispatch. CLEAN. +- `bus/activation.c` service activation dedup: uses `pending_activations` + hash table to coalesce duplicate requests. CLEAN. +- `bus/config-parser.c` service_dirs_find_dir: O(D^2) during config load but + D is small (< 20 dirs). LOW severity, startup only. CLEAN. +- `bus/services.c` owner list scan: O(N) per ownership transfer. N = number + of owners of a single service name, always very small. CLEAN. + +## MOAD-0002 (Intertangle): CLEAN + +`BusContext` in `bus/bus.h` is a well-scoped daemon context object holding +all bus state. This is the standard single-daemon pattern, not a god object +spanning independent subsystems. The sub-objects (BusPolicy, BusActivation, +BusRegistry, BusMatchmaker, BusConnections) have clean interfaces. CLEAN. + +## MOAD-0003 (Leaked Context): CLEAN + +dbus-daemon is single-threaded (main loop in `bus/main.c` using a custom +event loop). No `pthread_getspecific`, `thread_local`, or `__thread` in +`bus/`. CLEAN. + +## MOAD-0004 (CWE-312 Logged Secret): CLEAN + +All SASL auth logging (`dbus-auth.c`) uses `_dbus_verbose()` which compiles +to a no-op unless `DBUS_ENABLE_VERBOSE_MODE` is set at build time. Production +dbus binaries do not enable this flag. The `dbus-daemon-launch-helper` and +activation paths log only service names and error codes. CLEAN. + +## MOAD-0005 (Thundering Herd): CLEAN + +dbus-daemon is single-threaded. Service activation uses `pending_activations` +hash table to coalesce multiple concurrent activation requests for the same +service into a single launch, preventing herd behavior. CLEAN. diff --git a/defects/systemd-0004/TICKET.md b/defects/systemd-0004/TICKET.md new file mode 100644 index 000000000..cf0fc48d3 --- /dev/null +++ b/defects/systemd-0004/TICKET.md @@ -0,0 +1,72 @@ +# systemd-0004: seccomp-util.c strv_contains in KNOWN syscall scan O(K*A) + +**MOAD:** MOAD-0001 (CWE-407) +**Severity:** MEDIUM +**Speedup:** ~150x at K=537, A=300 (per architecture per service start) +**Language:** C +**File:** src/shared/seccomp-util.c:1195-1210 + +## Description + +`seccomp_load_syscall_filter_set()` builds a `char **added` string vector (strv) +containing every syscall name that was already handled by the requested filter +set. After processing the filter set it iterates over all ~537 syscalls in the +`@known` set and, for each one, calls `strv_contains(added, name)` to check +whether that syscall was already covered: + +```c +SECCOMP_FOREACH_LOCAL_ARCH(arch) { + _cleanup_strv_free_ char **added = NULL; + + r = add_syscall_filter_set(seccomp, set, action, NULL, log_missing, &added); + ... + NULSTR_FOREACH(name, syscall_filter_sets[SYSCALL_FILTER_SET_KNOWN].value) { + ... + if (strv_contains(added, name)) /* O(|added|) linear scan */ + continue; + ... + } +} +``` + +`strv_contains()` performs a linear scan. The `added` strv can hold 200-400 +entries depending on which filter set is loaded. On x86\_64 systemd iterates 3 +architectures (x86, x32, x86\_64). Total comparisons: + +``` +3 arches * 537 KNOWN * ~300 added = ~484,000 string comparisons per service start +``` + +The sibling function `seccomp_load_syscall_filter_set_raw()` (line 1246) +already uses `hashmap_contains(filter, ...)` for O(1) lookup, confirming that +O(1) lookup is the intended pattern. This function was never updated to match. + +Every systemd service unit that uses `SystemCallFilter=` triggers this path +on service activation. Container environments starting many services +simultaneously amplify the impact. + +## Fix + +Before the `NULSTR_FOREACH` loop, build a `Set *added_set` from `added` strv +entries. Use `set_contains(added_set, name)` instead of `strv_contains(added, +name)`. This mirrors the `hashmap_contains` pattern already used in the raw +variant. + +```c +/* Build O(1) lookup set from added strv */ +_cleanup_set_free_ Set *added_set = NULL; +STRV_FOREACH(n, added) { + r = set_put_strdup(&added_set, *n); + if (r < 0) + return log_oom(); +} + +NULSTR_FOREACH(name, syscall_filter_sets[SYSCALL_FILTER_SET_KNOWN].value) { + ... + if (set_contains(added_set, name)) /* O(1) */ + continue; + ... +} +``` + +Complexity drops from O(K * A) to O(K + A) per architecture. diff --git a/defects/systemd-0004/patch/SCAN.md b/defects/systemd-0004/patch/SCAN.md new file mode 100644 index 000000000..9aa01d57a --- /dev/null +++ b/defects/systemd-0004/patch/SCAN.md @@ -0,0 +1,38 @@ +# systemd deeper scan — 2026-04-03 + +Source: https://github.com/systemd/systemd (depth=1) +Focus: src/shared/seccomp-util.c (missed in 2026-03-31 scan) + +Prior defects: +- systemd-0001: strv_extend_strv dedup O(N^2) (src/basic/strv.c) — HIGH +- systemd-0002: unit_file_get_list states filter O(U*S) (src/shared/install.c) — MEDIUM +- systemd-0003: dbus-cgroup BPF filter strv dedup O(N^2) (src/core/dbus-cgroup.c) — LOW-MEDIUM + +## MOAD-0001 (CWE-407): DEFECT — systemd-0004 + +`src/shared/seccomp-util.c` `seccomp_load_syscall_filter_set()`: + +After processing the requested filter set (building `char **added` strv of +covered syscall names), the function iterates all ~537 KNOWN syscalls and for +each calls `strv_contains(added, name)` which is an O(|added|) linear scan. + +This produces O(K * A) comparisons where: +- K = 537 KNOWN syscalls on x86\_64 +- A = filter set size (200-400 for common sets like @default, @system-service) + +On x86\_64 systemd processes 3 architectures (x86, x32, x86\_64): +``` +3 * 537 * ~300 = ~484,000 string comparisons per service activation with SeccompFilter= +``` + +The sibling function `seccomp_load_syscall_filter_set_raw()` already uses +`hashmap_contains(filter, id)` for O(1) lookup. This function was left behind +when the raw variant was optimized. + +**Fix:** build a `Set *added_set` from the `added` strv before the +`NULSTR_FOREACH` loop, then use `set_contains(added_set, name)` for O(1). +Complexity drops from O(K*A) to O(K+A) per architecture. + +See `systemd-0004-seccomp-strv-to-set.patch` and `TICKET.md`. + +## MOADs 0002-0005: carried from 2026-03-31 scan (all CLEAN) diff --git a/defects/systemd-0004/patch/systemd-0004-seccomp-strv-to-set.patch b/defects/systemd-0004/patch/systemd-0004-seccomp-strv-to-set.patch new file mode 100644 index 000000000..f317e595f --- /dev/null +++ b/defects/systemd-0004/patch/systemd-0004-seccomp-strv-to-set.patch @@ -0,0 +1,44 @@ +# UNDF: UNDF-PENDING +--- a/src/shared/seccomp-util.c ++++ b/src/shared/seccomp-util.c +@@ -1178,7 +1178,7 @@ int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilter + + SECCOMP_FOREACH_LOCAL_ARCH(arch) { + _cleanup_(seccomp_releasep) scmp_filter_ctx seccomp = NULL; +- _cleanup_strv_free_ char **added = NULL; ++ _cleanup_strv_free_ char **added = NULL; ++ _cleanup_set_free_ Set *added_set = NULL; + + log_trace("Operating on architecture: %s", seccomp_arch_to_string(arch)); + +@@ -1188,10 +1188,20 @@ int seccomp_load_syscall_filter_set(uint32_t default_action, const SyscallFilter + if (r < 0) + return log_debug_errno(r, "Failed to add filter set: %m"); + ++ /* Build O(1) lookup set from the added strv to avoid O(K*A) quadratic ++ * scan in the NULSTR_FOREACH loop below. The sibling function ++ * seccomp_load_syscall_filter_set_raw() already uses hashmap_contains ++ * for O(1); this brings the named-set path in line with it. */ ++ if (default_action != default_action_override) { ++ char **n; ++ STRV_FOREACH(n, added) { ++ r = set_put_strdup(&added_set, *n); ++ if (r < 0) ++ return log_oom(); ++ } ++ } ++ + if (default_action != default_action_override) + NULSTR_FOREACH(name, syscall_filter_sets[SYSCALL_FILTER_SET_KNOWN].value) { + int id; + + id = sym_seccomp_syscall_resolve_name(name); + if (id < 0) + continue; + + /* Ignore the syscall if it was already handled above */ +- if (strv_contains(added, name)) ++ if (set_contains(added_set, name)) + continue; + + r = sym_seccomp_rule_add_exact(seccomp, default_action, id, 0); diff --git a/defects/systemd-0004/test/TestSeccompStrvToSet.java b/defects/systemd-0004/test/TestSeccompStrvToSet.java new file mode 100644 index 000000000..fea01f070 --- /dev/null +++ b/defects/systemd-0004/test/TestSeccompStrvToSet.java @@ -0,0 +1,117 @@ +package defects.systemd_0004; + +import org.junit.Test; +import static org.junit.Assert.*; + +import java.util.*; + +/** + * Unit test demonstrating systemd-0004 CWE-407: + * seccomp_load_syscall_filter_set() uses strv_contains (O(N) linear scan) + * inside NULSTR_FOREACH over ~537 KNOWN syscalls, producing O(K*A) behavior. + * + * The sibling function already uses hashmap/set for O(1) lookup. + * This test proves O(K*A) vs O(K+A) operation count at realistic scales. + */ +public class TestSeccompStrvToSet { + + /** Simulates strv_contains: linear scan of a list for a name. */ + static int strv_contains(List added, String name) { + for (String s : added) { + if (s.equals(name)) return 1; // found + } + return 0; // not found + } + + /** Simulates set_contains: O(1) hash lookup. */ + static int set_contains(Set added_set, String name) { + return added_set.contains(name) ? 1 : 0; + } + + /** + * Measures operation count for the O(K*A) defect vs O(K+A) fix. + * + * @param K number of KNOWN syscalls (e.g. 537 on x86_64) + * @param A number of syscalls already added to filter (e.g. 300 for @default) + */ + static long[] measureOps(int K, int A) { + // Build KNOWN syscall list (K entries) + List known = new ArrayList<>(); + for (int i = 0; i < K; i++) { + known.add("sys_" + i); + } + + // Build added strv: first A syscalls already covered + List added_strv = new ArrayList<>(); + Set added_set = new HashSet<>(); + for (int i = 0; i < A; i++) { + added_strv.add("sys_" + i); + added_set.add("sys_" + i); + } + + // Defect: O(K*A) - strv_contains inside loop over KNOWN + long ops_defect = 0; + for (String name : known) { + // strv_contains does up to A comparisons + for (String s : added_strv) { + ops_defect++; + if (s.equals(name)) break; + } + } + + // Fix: O(K+A) - set_contains inside loop over KNOWN + // Build set: A ops + long ops_fix = A; // set construction + // Loop: K * O(1) lookups + for (String name : known) { + ops_fix++; // one hash lookup per iteration + if (added_set.contains(name)) continue; + } + + return new long[]{ops_defect, ops_fix}; + } + + @Test + public void testRealisticScale() { + // K=537 (x86_64 KNOWN syscalls), A=300 (@default filter set size) + int K = 537, A = 300; + long[] ops = measureOps(K, A); + long defect = ops[0], fix = ops[1]; + + System.out.printf("K=%d, A=%d: defect=%,d ops, fix=%,d ops, speedup=%.1fx%n", + K, A, defect, fix, (double) defect / fix); + + // The defect should be dramatically more expensive + assertTrue("Defect should have > 10x more ops than fix at K=537, A=300", + defect > fix * 10); + + // Compute expected speedup + double speedup = (double) defect / fix; + assertTrue("Speedup should be > 50x at K=537, A=300", speedup > 50.0); + } + + @Test + public void testSmallScale() { + // Even at small scale (K=100, A=50) the defect shows clearly + long[] ops = measureOps(100, 50); + long defect = ops[0], fix = ops[1]; + System.out.printf("K=100, A=50: defect=%,d ops, fix=%,d ops, speedup=%.1fx%n", + 100, 50, defect, fix, (double) defect / fix); + assertTrue("Defect always more expensive than fix", defect > fix); + } + + @Test + public void testMultipleArches() { + // x86_64 runs 3 architectures (x86, x32, x86_64) + int arches = 3, K = 537, A = 300; + long total_defect = 0, total_fix = 0; + for (int i = 0; i < arches; i++) { + long[] ops = measureOps(K, A); + total_defect += ops[0]; + total_fix += ops[1]; + } + System.out.printf("3 arches: defect=%,d ops, fix=%,d ops, speedup=%.1fx%n", + total_defect, total_fix, (double) total_defect / total_fix); + assertTrue("3-arch total: defect > 100x fix", total_defect > total_fix * 100); + } +}