satellite-provider/_docs/05_security/static_analysis.md

# Phase 2 — Static Analysis (SAST)

**Date**: 2026-05-11
**Scope**: All `*.cs` files in production projects (Api, Common, DataAccess, Services.*) plus Tests for false-positive triage. Configuration files (`appsettings*.json`, `docker-compose*.yml`, `Dockerfile`, `.env`).
**Method**: Pattern-based grep + targeted file review.

## Patterns checked

| Category | Pattern(s) | Verdict |
|----------|-----------|---------|
| SQL injection | `$"SELECT…"`, `+ "WHERE"`, raw `CommandText`, manual SQL string assembly | **Clean** |
| Command/process injection | `Process.Start`, `ProcessStartInfo`, `cmd.exe`, `/bin/sh`, `UseShellExecute`, `eval`-equivalent | **Clean** |
| XSS | unsanitized user input flowed to HTML or `Response.Write` | **N/A** — JSON-only API, no HTML rendering |
| Template injection | Razor / scriban / handlebars on user input | **N/A** — none used |
| Hardcoded credentials | `password = "…"`, `secret = "…"`, `token = "…"`, `apikey = "…"` in source | See findings S1, S2 |
| Weak crypto | MD5/SHA1 for passwords, `RNGCryptoServiceProvider` (deprecated), hardcoded keys | **N/A** — no password storage, no crypto code in app |
| Insecure deserialization | `BinaryFormatter`, `pickle`, untrusted JSON with type-name handling | **Clean** — `System.Text.Json` with default settings; `Newtonsoft.Json` 13.0.4 used only for outbound serialization to Google session-creation endpoint (line `GoogleMapsDownloaderV2.cs`), no deserialization of untrusted inbound JSON |
| Path traversal | user input flowed into `File.Open`, `Path.Combine` | **Clean** — file paths are computed server-side from validated tile coordinates; no user-supplied path component reaches the filesystem |
| Sensitive data in logs | passwords, API keys, tokens, PII in log statements | **Clean** — `GlobalExceptionHandler.cs` logs only `Method`, `Path`, `correlationId`; client gets a generic 500 + correlationId. `CorsConfigurationValidator` warning (`PermissiveDefaultWarning`) does not include secrets. There is a deliberate test fixture `GlobalExceptionHandlerTests.cs:23` that uses `"Connection string Host=secret-db;Password=hunter2 failed at line 42"` to verify the handler does NOT echo exception messages back — this is a positive control, not a finding |
| Verbose error responses | stack traces or internal details returned to clients | **Clean** — `GlobalExceptionHandler` returns RFC 7807 ProblemDetails with `Detail = "An unexpected error occurred. Use the correlationId to look up the server log entry."` |
| Input validation | numeric ranges, geo coordinates, enum-like strings | See finding S3 |

## Findings

### S1 — Default DB password committed in `appsettings.json` (Medium)

- **Location**: `SatelliteProvider.Api/appsettings.json:24`
- **Vulnerable code**:
  ```json
  "DefaultConnection": "Host=localhost;Database=satelliteprovider;Username=postgres;Password=postgres"
  ```
- **Description**: The default (non-Development) appsettings file ships with a weak, well-known password (`postgres/postgres`). In production this string is overridden by `ConnectionStrings__DefaultConnection` in `docker-compose.yml`/env, but the file itself becomes the fallback if env-var injection ever fails or is misconfigured (silent connect-as-default behaviour).
- **Impact**: If a deployment misconfiguration drops the env override, the app silently falls back to attempting `postgres:postgres@localhost`. On a developer workstation this connects to the local Postgres container with full superuser; in production it would fail loudly only if the prod DB has different creds. Combined with finding S2 below (matching weak creds in compose file), this normalises a credential pattern that real production deployments may inherit.
- **Remediation**:
  - Replace the default value with a deliberately-invalid placeholder such as `Host=__set-via-env__;Database=__;Username=__;Password=__` so a misconfiguration fails fast at startup instead of silently falling through.
  - OR remove the `ConnectionStrings:DefaultConnection` key from `appsettings.json` entirely and require the env var; `Program.cs` line 23–24 already throws when missing — keep that behaviour.

### S2 — Weak Postgres credentials in `docker-compose.yml` (Medium, dev-only as written)

- **Location**: `docker-compose.yml:6-7, 30`
- **Vulnerable code**:
  ```yaml
  POSTGRES_USER: postgres
  POSTGRES_PASSWORD: postgres
  …
  - ConnectionStrings__DefaultConnection=Host=postgres;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres
  ```
- **Description**: Same `postgres/postgres` credentials as S1. The compose file is labelled `Development` (`ASPNETCORE_ENVIRONMENT=Development`), so this is contained — but the file is the only compose artifact in the repo, which means anyone running `docker-compose up` on a network-reachable host immediately exposes a Postgres-with-default-creds.
- **Impact**: Postgres on `0.0.0.0:5432` (port `"5432:5432"` mapping) with `postgres/postgres` is one of the most-scanned credential pairs on the public internet. If a developer runs this on a non-laptop host (cloud VM, shared lab, etc.) the DB is trivially compromised within minutes.
- **Remediation**:
  - Bind `5432` to `127.0.0.1:5432` rather than `0.0.0.0:5432` so the host firewall isn't the only protection. (Replace `"5432:5432"` with `"127.0.0.1:5432:5432"`.)
  - Source `POSTGRES_USER` / `POSTGRES_PASSWORD` from the same `.env` file that already supplies `GOOGLE_MAPS_API_KEY` (line 31 already shows the pattern). Provide an `.env.example` with placeholder values and document the required vars in the README.
  - The deploy/observability docs at `_docs/02_document/deployment/` already describe a secret-manager strategy for staging/prod — fold the same pattern into the dev compose.

### S3 — Latitude / longitude inputs not range-validated at the API boundary (Low)

- **Locations**:
  - `SatelliteProvider.Api/Program.cs:169` — `GetTileByLatLon([FromQuery] double Latitude, [FromQuery] double Longitude, [FromQuery] int ZoomLevel, …)`
  - `SatelliteProvider.Api/Program.cs:207` — `RequestRegion` validates `SizeMeters` only; `request.Latitude` / `request.Longitude` are unchecked
  - `SatelliteProvider.Api/Program.cs:237` — `CreateRoute` delegates to `RouteService` which validates names but does not range-check waypoint coordinates
- **Description**: `Latitude`, `Longitude`, and (for region requests) the implicit `MaxRoutePointSpacingMeters` boundary are accepted without enforcing valid geographic ranges (`-90 ≤ lat ≤ 90`, `-180 ≤ lon ≤ 180`). `ZoomLevel` IS validated downstream by `GoogleMapsDownloaderV2` against `MapConfig.AllowedZoomLevels` — so it is fine.
- **Impact**:
  - Garbage inputs (e.g. `lat=999`) propagate through `GeoUtils.WorldToTilePos` and the slippy-map math, eventually producing nonsensical tile coordinates that are persisted to `tiles` and `regions`. This is a **data-quality** issue, not a code-execution issue.
  - No DoS amplification: every tile-download endpoint already enforces zoom against `AllowedZoomLevels`, so an attacker cannot use lat/lon abuse to multiply outbound Google Maps traffic beyond what zoom already bounds.
- **Remediation**: Add explicit guard clauses at the API boundary (matches the existing `SizeMeters` 100-10000 pattern):
  ```csharp
  if (Latitude < -90 || Latitude > 90)  return Results.BadRequest(new { error = "Latitude must be between -90 and 90" });
  if (Longitude < -180 || Longitude > 180) return Results.BadRequest(new { error = "Longitude must be between -180 and 180" });
  ```
  Apply uniformly to `GetTileByLatLon`, `RequestRegion`, and to each waypoint inside `CreateRoute`.

### S4 — `.env` file on developer filesystem contains an apparently real Google Maps API key (Medium — exposure depends on key reach)

- **Location**: `.env` (workspace root, **not** tracked — confirmed via `git ls-files` and `.gitignore:10`)
- **Description**: The local `.env` contains a 39-character `AIzaSy…` value matching the Google Maps API key format. The file is correctly excluded from git (line 10 of `.gitignore`) and `git log -- .env` returns no history, so the key was never committed to this repository.
- **Impact**: No repository exposure. **However**:
  - If the same key is shared across developers via Slack / email / other repos, it has likely already leaked elsewhere.
  - There is no `.env.example` template in the repo, which means new contributors typically request the real key via insecure channels rather than generating a fresh one.
  - The key has no per-call attribution; abuse cannot be traced back to a specific developer.
- **Remediation**:
  - **Rotate the key in the Google Cloud console** (out of scope for this audit — the key value is intentionally not echoed into this report).
  - Add `.env.example` to the repo with `GOOGLE_MAPS_API_KEY=replace-with-your-own-key-from-cloud-console` and reference it in the README setup section.
  - Configure Google Cloud key restrictions: HTTP referrer allowlist (for browser keys) or IP allowlist (for server keys), and per-API quotas. Optional: per-developer keys.

## Self-verification

- [x] All production source directories scanned (Api, Common, DataAccess, Services.TileDownloader, Services.RegionProcessing, Services.RouteManagement)
- [x] Each finding has file path and line number
- [x] False positives from test files explicitly distinguished (`GlobalExceptionHandlerTests.cs:23` "leakySecret" is a positive control)
- [x] No real secret values printed in this report (S4 is described without echoing the key)