towerops/bugs.md

Bugs And Risk Findings

Review date: 2026-05-11

Scope: application-owned Elixir/Phoenix code, templates, config, deployment manifests, scripts, and dependency manifests. Vendored code and bundled third-party MIB data were not reviewed as first-party code.

Validation run:

• mix credo --strict: passed, no issues.
• mix deps.audit: passed, no vulnerabilities found.
• mix hex.audit: passed, no retired packages found.
• npm audit --prefix e2e --audit-level=low: passed, 0 vulnerabilities.

High

3. Admin MIB archive upload is vulnerable to archive bombs and symlink/path surprises

• Category: OWASP A05 Security Misconfiguration / A08 Software and Data Integrity Failures / Availability
• Evidence: lib/towerops_web/controllers/api/v1/mib_controller.ex:197, lib/towerops_web/controllers/api/v1/mib_controller.ex:244, lib/towerops_web/controllers/api/v1/mib_controller.ex:373
• Problem: uploaded .tar.gz and .zip files are extracted by external commands into temp storage without limits on archive size, extracted file count, total uncompressed bytes, nesting depth, or per-file size. validate_extracted_paths/1 checks expanded paths after extraction, but does not prevent resource exhaustion during extraction and does not reject symlinks before File.cp_r!/2.
• Fix: enforce upload body limits, archive entry count limits, max uncompressed bytes, max nesting depth, and reject symlinks/device files before copying. Prefer an archive reader that can inspect entries before writing, or run extraction in a constrained worker/container. Add tests for zip-slip, symlink, and oversized archive rejection.

4. MIB tokenizer creates atoms from untrusted file content

• Category: OWASP A04 Insecure Design / Availability
• Evidence: lib/snmpkit/snmp_lib/mib/snmp_tokenizer.ex:621, lib/snmpkit/snmp_lib/mib/snmp_tokenizer.ex:655, lib/snmpkit/snmp_lib/mib/parser.ex:535
• Problem: uploaded/imported MIB content is converted with String.to_atom/1. Atoms are not garbage collected on the BEAM, so a malicious or simply very large set of unique identifiers can exhaust the VM atom table and crash the node.
• Fix: keep tokenizer identifiers as strings or use a bounded atom allowlist with String.to_existing_atom/1 only for known reserved words. Add a regression test that parsing many unique identifiers does not grow the atom table.

Medium

6. CSP is duplicated and weakened by inline scripts

• Category: OWASP A05 Security Misconfiguration / A03 Injection defense in depth
• Evidence: lib/towerops_web/router.ex:21, lib/towerops_web/plugs/security_headers.ex:20, lib/towerops_web/components/layouts/root.html.heex:35, lib/towerops_web/components/layouts/root.html.heex:71, lib/towerops_web/components/layouts/root.html.heex:80
• Problem: CSP is set in both the browser pipeline and endpoint-level security plug, with different directives. The effective behavior depends on header overwrite order. The policy also requires 'unsafe-inline' because templates include inline scripts, reducing CSP's value against XSS.
• Fix: centralize CSP construction in one plug. Move inline scripts into assets/js/app.js or LiveView hooks and remove 'unsafe-inline' where possible. Consider nonces only for unavoidable inline bootstrapping.

9. Custom CSS is rendered from database into a raw style tag

• Category: OWASP A03 Injection / CSS injection
• Evidence: lib/towerops_web/live/status_page_live.html.heex:26
• Problem: @config.custom_css is rendered inside <style>. HEEx escapes HTML, but CSS itself can still be abused for UI redress, data exfiltration attempts through external URLs, or brand/content spoofing on public status pages if an attacker gains config write access.
• Fix: sanitize or constrain custom CSS to a safe subset, disallow external url(...) references, or store structured theme settings instead of arbitrary CSS.

10. Direct Repo calls in LiveView and LiveView helper modules violate context boundaries

• Category: Idiomatic Elixir / Phoenix architecture
• Evidence: lib/towerops_web/live/agent_live/edit.ex:19, lib/towerops_web/live/agent_live/edit.ex:21, lib/towerops_web/live/user_settings_live/totp_manager.ex:57, lib/towerops_web/live/user_settings_live/totp_manager.ex:61
• Problem: LiveViews and UI helpers call Towerops.Repo directly. That bypasses context APIs, scatters authorization/data-access rules, and violates the project guideline that Ecto access belongs in contexts.
• Fix: add context functions such as Agents.get_agent_token_for_edit!/2 and Accounts.verify_new_totp_device/3, then call those from LiveViews. Unit test the context functions, including authorization boundaries.

11. Nested API helpers fetch child records without organization in the query

• Category: OWASP A01 Broken Access Control / Performance / Idiomatic Ecto
• Evidence: lib/towerops_web/controllers/api/v1/schedules_controller.ex:356, lib/towerops_web/controllers/api/v1/schedules_controller.ex:363, lib/towerops_web/controllers/api/v1/schedules_controller.ex:370, lib/towerops_web/controllers/api/v1/escalation_policies_controller.ex:279, lib/towerops_web/controllers/api/v1/escalation_policies_controller.ex:286
• Problem: child resources are fetched by global ID and then checked against the parent ID in Elixir. The parent is scoped first, so this is not an immediate data leak, but the database does unnecessary global lookups and the pattern is easy to copy into a real authorization bug.
• Fix: move nested-resource lookups into the OnCall context and query with all available scope predicates in SQL, including organization via joins when possible.

12. Bang calls and exact matches can turn normal failures into request/process crashes

• Category: Reliability / Idiomatic Elixir
• Evidence: lib/towerops_web/live/agent_live/edit.ex:19
• Problem: several request/LiveView paths use {:ok, _} = ... or bang functions for operations that can fail because records disappear, DB writes fail, billing calls fail, or IDs are invalid. These become 500s or LiveView crashes instead of controlled error responses.
• Fix: replace bang/exact-match assumptions with case/with and user-appropriate responses. Keep bang functions for programmer invariants only.
• Partially fixed: schedules_controller, escalation_policies_controller, mobile_qr_live converted to case. settings_live kept as-is (dialyzer proves estimated_monthly_cost always returns {:ok, _}). agent_live Repo calls remain.

13. Process dictionary is used to pass cookie-consent UI state

• Category: Idiomatic Elixir / Maintainability
• Evidence: lib/towerops_web/user_auth.ex:671, lib/towerops_web/components/layouts.ex:50, lib/towerops_web/components/marketing_layouts.ex:28
• Problem: request/UI state is passed through Process.put/2 and Process.get/2. This is implicit, hard to test, and can produce surprising behavior as rendering moves between processes.
• Fix: pass requires_cookie_consent explicitly as an assign to layouts/components, or use a shared layout helper that reads from assigns with a default.

14. Admin Oban flush deletes jobs directly instead of using Oban APIs

• Category: Data integrity / Idiomatic library usage
• Evidence: lib/towerops_web/controllers/admin_controller.ex:27, lib/towerops_web/controllers/admin_controller.ex:40
• Problem: the admin endpoint deletes rows directly from oban_jobs. Direct deletion bypasses Oban's cancellation semantics, telemetry, plugins, and any cleanup hooks. It can also race with producers.
• Fix: use Oban cancellation/draining APIs for supported states, or isolate this as an explicitly documented emergency-only operation with audit logging and confirmation.

Low

15. API token rate limiting is only per IP, not per token/organization

• Category: OWASP A04 Insecure Design / Abuse prevention
• Evidence: lib/towerops_web/plugs/rate_limit.ex:55
• Problem: API routes rate-limit by IP before token identity is considered. A noisy token behind NAT can affect other users, and a single token can distribute requests across IPs to exceed intended quotas.
• Fix: after authentication, add token- or organization-level limits for API routes. Keep IP limits as an outer abuse-control layer.

16. MIB vendor listing counts files synchronously on request

• Category: Performance
• Evidence: lib/towerops_web/controllers/api/v1/mib_controller.ex:350, lib/towerops_web/controllers/api/v1/mib_controller.ex:373
• Problem: each GET /admin/api/mibs request recursively walks the whole MIB directory with Path.wildcard("**/*"). This can be expensive with the large MIB tree already present in the repo and can block request processes under load.
• Fix: cache counts, keep metadata in the database, or compute counts asynchronously.

17. Debug headers route exists in production admin scope

• Category: OWASP A05 Security Misconfiguration / Information disclosure
• Evidence: lib/towerops_web/router.ex:292
• Problem: /admin/headers is superuser-protected, but still exposes request/header debugging functionality in the production admin surface.
• Fix: gate it behind dev_routes, remove it, or require an explicit runtime flag.