towerops/docs/security-analysis.md

# Security Analysis and Mitigations

This document tracks Sobelow security findings and the mitigations applied to address them.

## Summary

As of 2026-02-01, the following high-confidence security issues have been addressed:

- ✅ **Config.CSP**: Content-Security-Policy headers configured
- ✅ **Config.HTTPS**: HTTPS enforcement enabled in production
- ✅ **Traversal.FileModule**: Directory traversal protections added to MIB controller
- ⚠️ **Vendored Libraries**: Low-risk warnings in SnmpKit (vendored library)

---

## Fixed Issues

### 1. Config.CSP: Missing Content-Security-Policy (FIXED)

**Severity**: High
**Location**: `lib/towerops_web/router.ex:19`
**Status**: ✅ Fixed

**Mitigation**:
Added Content-Security-Policy headers to the `:browser` pipeline with LiveView-compatible settings:

```elixir
plug :put_secure_browser_headers, %{
  "content-security-policy" =>
    "default-src 'self'; " <>
    "script-src 'self' 'unsafe-inline' 'unsafe-eval'; " <>
    "style-src 'self' 'unsafe-inline'; " <>
    "img-src 'self' data: https:; " <>
    "font-src 'self' data:; " <>
    "connect-src 'self' ws: wss:; " <>
    "frame-ancestors 'none';"
}
```

**Note**: `'unsafe-inline'` for scripts is required for Phoenix LiveView to function. This is a documented requirement for LiveView applications.

---

### 2. Config.HTTPS: HTTPS Not Enabled (FIXED)

**Severity**: High
**Location**: `config/prod.exs`
**Status**: ✅ Fixed

**Mitigation**:
Configured `force_ssl` in production endpoint configuration (`config/runtime.exs`):

```elixir
config :towerops, ToweropsWeb.Endpoint,
  force_ssl: [
    hsts: true,
    rewrite_on: [:x_forwarded_host, :x_forwarded_port, :x_forwarded_proto]
  ]
```

**Security Benefits**:
- Automatically redirects HTTP → HTTPS
- Enables HSTS (HTTP Strict Transport Security)
- Respects X-Forwarded-* headers from reverse proxy (Traefik)

---

### 3. Traversal.FileModule: Directory Traversal in MIB Controller (FIXED)

**Severity**: High
**Locations**:
- `lib/towerops_web/controllers/api/v1/mib_controller.ex:119` (File.rm_rf)
- `lib/towerops_web/controllers/api/v1/mib_controller.ex:229` (File.cp)

**Status**: ✅ Fixed

**Mitigation**:
Added comprehensive input validation to prevent directory traversal attacks:

#### Vendor Name Validation
```elixir
defp validate_vendor_name(vendor) when is_binary(vendor) do
  # Reject path traversal characters
  if String.contains?(vendor, [".", "/", "\\", ":"]) do
    {:error, "Invalid vendor name: cannot contain path separators or dots"}
  else
    # Only allow alphanumeric, hyphen, underscore
    if vendor =~ ~r/^[a-zA-Z0-9_-]+$/ do
      :ok
    else
      {:error, "Invalid vendor name: must contain only letters, numbers, hyphens, and underscores"}
    end
  end
end
```

#### Filename Validation
```elixir
defp validate_filename(filename) when is_binary(filename) do
  # Reject directory traversal sequences
  if String.contains?(filename, ["..", "/", "\\"]) or String.starts_with?(filename, ".") do
    {:error, "Invalid filename: cannot contain path separators or parent directory references"}
  else
    :ok
  end
end
```

**Additional Protections**:
- All vendor names validated before path construction
- Filenames sanitized with `Path.basename/1`
- Superuser-only API access required for all MIB management endpoints
- All operations logged for audit trail

---

## Remaining Low-Risk Findings

### Vendored Library Warnings (SnmpKit)

**Severity**: Low Confidence
**Status**: ⚠️ Accepted Risk

The following warnings are in the vendored `SnmpKit` library (`lib/snmpkit/`):

#### Misc.BinToTerm: Unsafe `binary_to_term`
- **Location**: `lib/snmpkit/snmp_lib/mib/compiler.ex:203`
- **Risk Level**: Medium-High
- **Reason**: Used for loading compiled MIB files from trusted sources only
- **Mitigation**: MIB files are only loaded from the application's `priv/mibs/` directory, which is controlled by administrators. No user-provided binary data is deserialized.

#### DOS.StringToAtom / DOS.ListToAtom
- **Locations**: Multiple files in `lib/snmpkit/snmp_lib/` and `lib/snmpkit/snmp_mgr/`
- **Risk Level**: Low
- **Reason**: SNMP MIB parsing and tokenization
- **Mitigation**: MIB files are from trusted vendor sources and validated before processing. Atom creation is bounded by the finite set of SNMP OID names in standard MIBs.

#### Traversal.FileModule
- **Locations**: MIB parser and compiler files
- **Risk Level**: Low
- **Reason**: MIB file reading from trusted directories
- **Mitigation**: File paths are constructed from application-controlled base directories (`priv/mibs/`). User input does not directly influence file paths.

#### SQL.Query: SQL injection
- **Locations**: `lib/towerops/monitoring.ex` (multiple functions)
- **Risk Level**: Very Low (False Positive)
- **Reason**: Uses parameterized queries with string interpolation only for table/column names
- **Mitigation**: Table names are hardcoded constants, not user input. All user values use proper `$1`, `$2` parameter binding.

---

## Ignored Low-Confidence Warnings

The following warnings have been reviewed and determined to be false positives:

### XSS.Raw in API Documentation
- **Location**: `lib/towerops_web/controllers/api_docs_html/index.html.heex`
- **Reason**: Uses `raw(~S"""...""")` to display static code examples in API documentation
- **Risk**: None - all content is static, developer-authored code examples
- **No Action Required**

---

## Security Best Practices

### File Upload Security
1. ✅ Validate all filenames and vendor names
2. ✅ Use `Path.basename/1` to strip directory components
3. ✅ Restrict operations to predefined base directories
4. ✅ Require superuser authentication for file management
5. ✅ Log all file operations for audit trail

### HTTPS Configuration
1. ✅ Force SSL with HSTS enabled
2. ✅ Respect X-Forwarded-* headers from reverse proxy
3. ✅ Use secure cipher suites (default Phoenix configuration)

### Content Security Policy
1. ✅ Default to 'self' for all resources
2. ✅ Allow WebSocket connections for LiveView
3. ✅ Prevent framing with `frame-ancestors 'none'`
4. ⚠️ Allow `'unsafe-inline'` scripts (required for LiveView)

---

## Testing Recommendations

### Manual Security Testing
- [ ] Test MIB upload with path traversal attempts (`../../../etc/passwd`)
- [ ] Verify HTTPS redirect in production environment
- [ ] Test CSP headers don't break LiveView functionality
- [ ] Verify file upload restrictions (filename validation)

### Automated Testing
- Run `mix sobelow` regularly in CI/CD pipeline
- Monitor for new security vulnerabilities in dependencies
- Use Dialyzer for type safety checks

---

## Future Improvements

1. **Stricter CSP**: Explore using nonces for scripts instead of `'unsafe-inline'` (requires LiveView 1.0+)
2. **File Upload Limits**: Add file size limits for MIB uploads
3. **Rate Limiting**: Implement rate limiting on API endpoints
4. **Security Headers**: Add additional security headers (Permissions-Policy, etc.)
5. **Dependency Scanning**: Set up automated dependency vulnerability scanning

---

## References

- [Sobelow Security Guide](https://github.com/paraxialio/sobelow_guide)
- [Phoenix Security Best Practices](https://hexdocs.pm/phoenix/security.html)
- [OWASP Top 10](https://owasp.org/www-project-top-ten/)
- [Content Security Policy Reference](https://content-security-policy.com/)