# API Connect — Security & Effectiveness Review **Date:** 2026-07-01 **Reviewer:** Code review (automated, assisted) **Scope:** Full application — routing, authentication (external browser SSO), DB layer, backend API endpoints, page templates, WebSocket chat server, secret management. **Repository state:** branch `main` @ `2a10592` > ⚠️ This review found **critical issues that warrant immediate action** — most urgently, live secrets (including the master encryption key) committed to the git repository, and the absence of authentication on the majority of API endpoints. See the [Immediate action](#immediate-action-first-72-hours) checklist. --- ## Executive summary The application has a sound *cryptographic foundation* (libsodium authenticated encryption, blind-index salts) and the **newer code is written carefully** — recent files consistently authenticate, quote SQL values, whitelist tables, and escape output. However, a large body of **older endpoints predate those conventions** and ship with no authentication, no authorization checks, and direct object references. Combined with **secrets checked into version control** and **weak session-cookie hardening**, the current state exposes essentially all ticket, user, document, and call data to anyone who can reach the site. The authentication model itself (browser-mediated SSO with a shared-key encrypted token) is reasonable in design, but its enforcement is inconsistent: the session token never expires, cookies are readable by JavaScript, and only ~24% of API endpoints verify the session at all. | Severity | Count | Examples | |---|---|---| | 🔴 Critical | 3 | Secrets in git, missing auth on ~135 endpoints, decryption oracle | | 🟠 High | 9 | Confirmed SQL injection, stored XSS (ticket text + chat), cookie hardening, no CSRF, non-expiring tokens, WebSocket auth, token leakage, unauth info disclosure | | 🟡 Medium | 8 | SQL identifier injection design, JS-context injection, open redirect, weak hash, .htaccess-only protection, CSP disabled, no rate limiting, no prepared statements | | ⚪ Low / maintainability | 7 | `/vars` config dump, `strftime` removal in PHP 9, error suppression, dead code, IP-gated debug, stale data | > Two automated taint sweeps (SQL injection, XSS) confirmed a live SQL injection in `editableSave.php` reachable by any logged-in user, and multiple stored cross-site scripting vectors in ticket text and chat that fire for every agent/manager who opens the record. These are folded into the High section below (H7–H9). --- ## Immediate action (first 72 hours) 1. **Rotate every secret in `enc/` and in source.** They are in git history, so deletion alone is insufficient. This includes: - `enc/key-enc.txt` — the master libsodium key protecting *all* tokens and encrypted DB fields. Rotating this requires a re-encryption plan for existing ciphertext. - `enc/salt-bidx.txt`, `enc/AuthKey*.p8`/`.pem` (Apple), `enc/api-connect-...firebase-adminsdk...json` (Firebase admin), `enc/gorush-config.yml`. - Production DB password in `code/site.inc.php`, SMTP password in `backend/messageSupport.php`, OpenObserve credentials in `code/error_functions.inc.php`. 2. **Gate the entire `/do/` router behind `check_auth_json()`** by default (allowlist the few endpoints that must stay public), rather than relying on each endpoint to remember to check. 3. **Delete or lock down `backend/decrypt.php`** — it is an unauthenticated decryption oracle. 4. **Purge secrets from git history** (e.g. `git filter-repo`) and force-rotate, then move secrets to environment variables / a secrets manager outside the web root and outside the repo. --- ## 🔴 Critical findings ### C1. Live secrets committed to the repository (including the master encryption key) The following are tracked in git and present in history since the initial commit: - `enc/key-enc.txt` — the 32-byte libsodium `secretbox` key (`$SITE['DB_KEY_ENC']`). This single key protects: the `atoken`/`xtoken` auth cookies, `chatToken`, all `db_encrypt`-ed database fields, and the SSO token exchange. - `enc/salt-bidx.txt` — blind-index salt. - `enc/AuthKey.pem`, `enc/AuthKey_D5JW82Z474.p8` — Apple push/authentication keys. - `enc/api-connect-b8632-firebase-adminsdk-jucql-*.json` — Firebase admin service account (full project admin). - `enc/gorush-config.yml` — push gateway config. - `code/site.inc.php` — hardcoded production MariaDB credentials (`connect` / `NJ95325rp5i2!sr5`) and the news DB creds, plus WebPush VAPID keys. - `backend/messageSupport.php` — SMTP username/password (`cpv@bmwdealerforum.org` / `SunnyW00dbridge!`). - `code/error_functions.inc.php` — OpenObserve basic-auth credentials as function default arguments. **Impact:** Anyone with read access to the repo (or any leaked clone/backup) can **forge authentication tokens for any user, decrypt all encrypted data, send push/email as the org, and connect directly to the production database.** This is the highest-priority item. **Fix:** Rotate all of the above. Move secrets out of the repo (env vars / secrets manager, loaded at runtime). Add `enc/` and any secret files to `.gitignore` *and* purge them from history. Treat the master key rotation as a data-migration project (decrypt-with-old / re-encrypt-with-new). > Note: `.htaccess` does block web access to `/enc` and `/.git` via `RedirectMatch 404`, which prevents *direct HTTP download*. It does **not** address repo/history exposure, and it is a fragile control (see M4). --- ### C2. Missing authentication and authorization on most backend endpoints The router (`code/route.inc.php`, `/do/{file}` branch) simply `include`s `backend/{file}.php` with **no global auth gate**. Each endpoint is responsible for calling `check_auth_json()` itself — and **135 of 177 endpoints (~76%) never do.** Confirmed exploitable examples (no auth, direct object reference): - `backend/ticketSet.php` — **writes** any ticket's status, agent, severity, issue/resolution text, timestamps, given `?ticket=`. No auth, no ownership check. - `backend/ticketGet.php` — reads any ticket's fields (including contact PII) by `pk_ticket`. - `pages/download.php` (route `/download/{id}`) — streams any document by `pk_doc`; no auth, no ownership check → IDOR over all uploaded files. - `backend/recordData.php` — unauthenticated write into the `records` table from request input. - Many `ticketManagerGet*`, `ticketGetLists`, `journalGet/Set`, `productsSubmit`, `alertsCreate`, etc. **Impact:** Full read/write access to tickets, documents, journals, and reports without a session. IDOR throughout because objects are addressed by sequential primary keys. **Fix:** Make authentication the default at the router: call `check_auth_json()` in the `/do/` dispatcher before including the handler, with an explicit small allowlist of intentionally-public endpoints (e.g. `healthcheck`, webhooks with their own signature verification, `loginProcess`, `loginCheck`). Add per-object authorization checks (does this user's system/permissions cover this `pk_ticket`/`pk_doc`?). Do the same for page routes via `check_auth_redirect()`. --- ### C3. Unauthenticated decryption oracle `backend/decrypt.php`: ```php $output['dec'] = db_decrypt($_REQUEST['enc']); ``` Reachable at `/do/decrypt?enc=...` with no authentication. It decrypts arbitrary attacker-supplied ciphertext with the master key and returns the plaintext. **Impact:** Any captured token or encrypted field (e.g. a `chatToken`, an `atoken`) can be submitted and read back in cleartext. It is a general-purpose oracle against the app's own crypto. **Fix:** Delete this endpoint (it appears to be a debugging tool). If a decrypt utility is genuinely needed, restrict to authenticated admins and never accept arbitrary ciphertext from the client. --- ## 🟠 High findings ### H1. Session/auth cookies are not hardened In `cw_include.inc.php` and `backend/loginProcess.php` / `code/functions.inc.php`, the auth cookies (`atoken`, `ssotoken`, `xtoken`) and the PHP session are set with: - `httponly => false` — **JavaScript can read the auth cookies.** Any XSS becomes full account takeover. - **No `SameSite` attribute** — enables cross-site request forgery (see H2). - `session_set_cookie_params(60*60*24*100, "/")` — **100-day** session lifetime. - `secure` only true in prod (acceptable), but combined with the above the exposure is large. **Fix:** Set `httponly => true` on all auth cookies (the JS never needs to read them server-side auth suffices), `samesite => 'Lax'` (or `Strict`), a much shorter session lifetime with sliding renewal, and `secure => true` everywhere HTTPS is available. ### H2. No CSRF protection on state-changing endpoints The `CSRF` class (`code/php-csrf.php`) is only wired into the legacy login (`pages/login.php`, `backend/old/*`). None of the 177 `/do/` action endpoints validate a CSRF token. With cookies lacking `SameSite` (H1), an attacker page can drive authenticated state changes (close tickets, edit records, register device tokens, etc.). **Fix:** Require a CSRF token (double-submit or synchronizer) on all non-idempotent `/do/` calls, or at minimum enforce `SameSite=Lax` cookies plus an origin/referer check in the dispatcher. ### H3. Auth tokens never expire `encrypt_package()` stamps `pts = time()`, but `decrypt_package()` / `check_auth()` **never validate it.** `backend/generateToken.php` *does* enforce a 5-hour `pts` window — showing the pattern exists but wasn't applied to the session path. A captured `xtoken`/`atoken` is valid indefinitely (cookie set for 30 days, token itself forever). **Fix:** Validate `pts` (and honor `xtoken_expires`) in `check_auth()`; reject stale tokens and force re-auth. ### H4. WebSocket chat server authentication is disabled/weak `chat/server.php`: - `command_authWorker()` — the token check is **commented out**; any connection can declare itself a privileged `worker`, drain the request queue, and relay/inject `response`/`send` messages to arbitrary users and tickets. - `command_authUser()` — trusts `pk_sso`/`sso` decoded from the token with the **server-side SSO verification commented out** and no expiry check. - `encrypt()`/`decrypt()` use a **hardcoded key `"Mhall1040"`** with AES-128-CTR (legacy, but present). - The live `chatMessageSend` path broadcasts `message['content']` **without** `htmlspecialchars`, while the history path escapes it — inconsistent, and a stored-XSS vector if the client renders with `.html()`. The server binds to `127.0.0.1:9895` (good — not directly public), but it is proxied at `wss://connect.apinet.com/chat`, so reachability should be assumed. **Fix:** Re-enable worker and user authentication with server-side token verification and expiry; remove the hardcoded-key cipher; escape all broadcast content consistently on output. ### H5. Sensitive token / cookie leakage to logs and email - `backend/messageSupport.php` includes the user's **`ssotoken` in a plaintext email** sent to `charles@apinet.com` on every "App Message." Session tokens should never be emailed. - `code/route.inc.php` writes full `$_REQUEST` and `$_COOKIE` (including tokens) to `cache/debug-cookies.{date}.json` for a hardcoded IP, and dumps `chatTokenDebug.json` on `/meeting-chat`. **Fix:** Remove token fields from support emails and remove the debug dumps (or redact and gate behind a dev-only flag). Ensure `cache/` is never web-served (currently blocked by `.htaccess` only — see M4). ### H6. Unauthenticated information disclosure / cache control - `backend/healthcheck.php` returns DB name, DB user, hostname, server version, and connection stats with no auth. - `backend/systemsCreateCache.php` lets anyone regenerate `cache/systems.json`. - `backend/throw.php` deliberately throws (error-path probing). **Fix:** Authenticate or IP-restrict healthcheck/ops endpoints; remove `throw.php` from production. ### H7. Confirmed SQL injection in `editableSave.php` (any authenticated user) `backend/editableSave.php` takes `$field = $_REQUEST['field']` (line 6) and passes it as an array **key** to `set_item($table, [$field => $newValue], ...)` (line 227). In `cw_database.inc.php` (`set_item`, lines 301–311) the key is concatenated raw into `sprintf("%s=%s", $name, $value)` with **no escaping or whitelisting**. `$table` is whitelisted by a `switch`, but `$field` is not. The endpoint calls `check_auth_json()` with **no permission argument**, so any logged-in user can reach it via `/do/editableSave`. **Exploit sketch:** `POST /do/editableSave` with `table=projects&id=1&type=text&field=name=(SELECT ...),other_col=1 -- &newValue=x` injects arbitrary additional SET expressions / subqueries into the UPDATE. A `field` value ending in `=` also disables value quoting, giving raw value injection as well. **Impact:** Arbitrary column writes and subquery injection against any whitelisted table, by any authenticated user — privilege escalation and data tampering. **Fix:** Whitelist `$field` against an allowed column list per `$table` (mirror the existing `$table` switch) before line 227; add a proper permission check. Longer term, move the wrapper to bound parameters (M1). *(The sibling `editableGet.php` uses `$field` only as a PHP array index, so it is not injectable.)* ### H8. Stored XSS in ticket issue / resolution / internal notes `format_content()` (`code/cw_layout.inc.php:4`) is a wiki-style formatter that emits raw HTML and **never escapes its input** — a line beginning with `<` even bypasses paragraph wrapping. User-submitted ticket text flows into it unescaped through two sinks: - **Server-rendered:** `pages/ticket/edit.php:515` (issue), `:542` (resolution), `:567` (internal notes); and `pages/ticket/status.php:120,137`. - **Client-side (`.html()`):** `backend/ticketSet.php:358/369/432` returns `format_content(trim($_REQUEST['text']))` in JSON, injected via `$("#textIssue").html(data['messageIssue'])` at `pages/ticket/edit.php:2451/2456/2460`. **Impact:** An issue body such as `` executes for every agent/manager who opens the ticket — persistent, cross-user. Combined with H1 (JS-readable auth cookies), this is account takeover. **Fix:** HTML-escape user text before/inside `format_content()`; render the client sinks with `.text()` or pre-escaped server output. ### H9. Stored XSS in ticket / meeting chat messages `backend/ticketSendChat.php:5` stores `$_REQUEST['content']` unmodified; `backend/ticketGetChatHistory.php:22` returns it raw; it is rendered with `.html()` at `pages/ticket/edit.php:1366` and `pages/meeting/chat.php:102`. (The adjacent sender/date fields correctly use `.text()` — only `content` is unsafe. The live WebSocket broadcast path has the same gap — see H4.) **Impact:** Persistent XSS fired at every chat participant. **Fix:** Escape `content` on output (server-side, or `.text()` on the client) consistently across live and historical paths. --- ## 🟡 Medium findings ### M1. SQL identifier injection is possible by design in the DB wrapper `code/cw_database.inc.php` builds SQL by string concatenation. Values pass through `PDO::quote()`, but **identifiers do not**: table names, column names (array keys), and the `orderby` / `groupby` / `having` / `field` / `limit` / `offset` pseudo-params are concatenated raw. A key ending in `=` also **disables quoting of its value** (raw injection). This is safe only as long as every caller keeps user input out of those positions. The taint sweep confirmed **one live instance** (H7, `editableSave.php`) plus a fragile-but-currently-safe pattern in `set_prop()` (`code/functions.inc.php:311`), where `'ip_addr='` injects `$_SERVER['REMOTE_ADDR']` raw into the SQL. `REMOTE_ADDR` is the socket peer address (not a client header) under a standard Apache setup, so it is not attacker-controllable today — but it would become injectable if a future reverse-proxy config ever populated `REMOTE_ADDR` from `X-Forwarded-For`. Recent code (`pages/ticket/phone-search.php`, `manager_calls.php`, `searchLocations.php`, `ticketGetLists.php`) was verified safe — inputs sanitized, all values `->quote()`d or integer-cast, identifiers/`orderby` constant. **Fix:** Migrate to prepared statements with bound parameters; whitelist/validate identifiers and `orderby` against known-good column lists inside the wrapper. Replace the `ip_addr=` raw pattern with `INET_ATON(?)` binding. ### M2. Open redirect(s) - `backend/loginProcess.php` builds `redirect = "/" . substr($_REQUEST['r'], 1)`. Input `r=//evil.com` yields `//evil.com`, a protocol-relative redirect off-site after login. - `code/route.inc.php:205` and `:477` do `header("Location: {$notif['link']}")` from a DB-stored notification link with no validation. The link is system-generated today (low risk), but should still be constrained to a relative path in case a link value is ever attacker-influenced. **Fix:** Validate that redirects are same-site absolute paths (start with a single `/`, not `//` or a scheme). ### M8. Wrong encoder for JavaScript string contexts `pages/meeting-room.php:88–89` emits request-supplied values into a `` in the value terminates the block and enables script injection (reflected). Same weak pattern at `:98` and `:191`. **Fix:** Emit JS values with `json_encode()` (which produces a safe quoted literal, including the closing-tag escape), not `htmlspecialchars` inside quotes. ### M3. Weak hashing primitive `db_hash()` uses `md5("APIbmwapp" . $string)` with a static, in-repo salt. If used for any security purpose (tokens, lookups against secrets), it is inadequate. **Fix:** Confirm usage; replace with HMAC-SHA-256 (keyed) or the blind-index scheme already present. ### M4. Protection of sensitive paths depends only on `.htaccess` `/enc`, `/.git`, `/cache`, `/vendor`, composer files are blocked via Apache `RedirectMatch`/rewrite. If `AllowOverride` is off, the file is missing, or the site is ever served by another front-end, these become publicly downloadable (including the master key). **Fix:** Move `enc/`, `cache/`, and secrets **outside the web root** so protection doesn't depend on server config. ### M5. Content-Security-Policy is disabled A per-request `script_nonce` is generated and the CSP header is written but **commented out** in both `route.inc.php` and `.htaccess`. The app therefore has no CSP despite the plumbing being ready. `X-XSS-Protection` (deprecated, can introduce issues) is set instead. **Fix:** Enable a nonce-based CSP; drop `X-XSS-Protection`. ### M6. No rate limiting Login, `messageSupport`, `userInvite`, and the unauthenticated endpoints have no throttling — enabling brute force, spam, and enumeration. **Fix:** Add rate limiting at the app or reverse-proxy layer for auth and public write endpoints. ### M7. Persistent connections + `query()` instead of prepared statements `ATTR_PERSISTENT => true` with a shared account and ad-hoc `->query()` string SQL couples the injection risk (M1) with connection-exhaustion and cross-request state concerns. **Fix:** Prepared statements (also addresses M1); reconsider persistent connections under load. --- ## ⚪ Low / maintainability - **L0. `/vars` (`pages/variables.php`) dumps `$_COOKIE`, `$SITE`, and `$_SESSION`** as `json_encode` inside `
`. It is auth-gated, but it exposes the full site config and session, and `json_encode` doesn't escape `<`, so a crafted cookie could break out of the `
`. Restrict to admins or remove.
- **L1. `strftime()` is used pervasively** (site, functions, reports, chat server). It is deprecated in PHP 8.1 and **removed in PHP 9.0**; the project targets "PHP 8.5+". This will break on upgrade. Migrate to `DateTime`/`date()`/`IntlDateFormatter`.
- **L2. Widespread `@` error suppression** (`@db_decrypt`, `@json_decode`, etc.) hides failures and complicates debugging.
- **L3. Production debug toggle via a filesystem file + IP match** (`/home/charles/debug` in `site.inc.php`) is fragile and can enable `display_errors` in prod.
- **L4. Dead/legacy code** (`backend/old/`, `*_beta.php`, large commented blocks) enlarges the attack surface and obscures the real flow.
- **L5. Stale data:** `$SITE['holidays']` only runs through 2024 — SLA/aging calculations relying on it are now wrong.
- **L6. `.well-known/apple-app-site-association` and other outputs** are fine, but overall the mix of old and new conventions makes it hard to reason about which endpoints are safe.

---

## What's working well

- **Authenticated encryption** via libsodium `secretbox` (nonce + MAC) for tokens and fields — the right primitive, correctly used in `enc_functions.inc.php`.
- **Blind-index salt** (`salt-bidx.txt`) indicates deliberate searchable-encryption design.
- **Recent code is solid:** `pages/ticket/phone-search.php`, `manager_calls.php`, `backend/uploadFile.php`, and `backend/editableSave.php` authenticate, quote values, whitelist tables, escape output with `htmlspecialchars`, and validate uploads by extension whitelist (`pdf/docx/xlsx/pptx`) while storing files without an executable extension.
- **Security headers** present in `.htaccess` (HSTS, `X-Frame-Options`, `nosniff`, `Referrer-Policy`), and directory indexing disabled.
- **The SSO design is reasonable:** shared-key authenticated tokens issued by the SSO server, exchanged through the browser — the weaknesses are in *enforcement* (expiry, cookie flags, per-endpoint checks), not the core concept.

---

## Suggested remediation roadmap

**Week 1 — contain**
1. Rotate all secrets (C1); move them out of the repo and web root; purge history.
2. Add the router-level auth gate + public allowlist (C2). Remove `decrypt.php` and `throw.php` (C3, H6).
3. Harden cookies: `HttpOnly`, `SameSite`, shorter lifetime (H1).

**Week 2–3 — enforce**
4. Patch the confirmed SQL injection in `editableSave.php` (H7) — whitelist `$field` + add permission check.
5. Fix stored XSS: escape ticket text and chat content on output; switch `.html()` sinks to `.text()` (H8, H9); fix the JS-context encoder in `meeting-room.php` (M8).
6. Enforce token `pts` expiry (H3); add CSRF protection (H2).
7. Fix the WebSocket auth and content escaping (H4); stop emailing/logging tokens (H5).
8. Add per-object authorization (ownership/system/permission checks) to ticket/document/record endpoints.

**Month 2 — harden & modernize**
9. Migrate the DB layer to prepared statements and identifier whitelisting (M1, M7).
10. Enable nonce-based CSP (M5); add rate limiting (M6); fix open redirects (M2).
11. Replace `strftime` before any PHP 9 upgrade (L1); remove dead code; refresh the holidays table; restrict `/vars` (L0, L4, L5).

---

## Method & coverage

- Reviewed core flow end-to-end: `index.php` → `route.inc.php` → auth (`functions.inc.php` `check_auth*`, `enc_functions.inc.php`) → DB layer (`cw_database.inc.php`) → representative backend handlers and page templates, plus the WebSocket server (`chat/server.php`) and Apache config (`.htaccess`).
- Enumerated auth coverage across all 177 backend endpoints.
- Verified secret exposure against git history.
- Targeted SQL-injection and XSS taint sweeps across `backend/` and `pages/` complemented the manual review. They confirmed one live SQL injection (H7), fragile-but-safe patterns (M1), and multiple stored/reflected XSS vectors (H8, H9, M8), and verified that the newer query-building code is safe.
- **Not covered (recommended next steps):** authenticated dynamic testing / DAST against a staging instance; a dependency audit (`composer audit` / npm) against `composer.lock` and `package-lock.json`; review of the SSO server itself (out of scope here); and server/Apache configuration hardening verification in production.