# IMS — Source Code Security Audit Report

| | |
|---|---|
| **Scope** | `IMS-Backend-New` (Laravel 12 / PHP 8.2, JWT auth, PostgreSQL) and `IMS-Frontend-New` (Next.js 16 / React 19) |
| **Type** | Static application security review + source-level remediation (white-box, full repository access) |
| **Standard** | OWASP Top 10 (2021) + OWASP API Security Top 10, mapped per finding below |
| **Companion documents** | `doc/security/server-hardening-guide.md` (infrastructure), `doc/security/payment-integrity-finding.md` (open Critical, detailed handoff) |
| **Result** | 32 findings raised · **23 fixed and verified** · 9 require a deploy/infra action or product-owner decision (0 left unaddressed in code) |

---

## 1. Executive summary

A full source-level review of both repositories found and remediated **six
Critical, sixteen High, eight Medium, one Low and one Informational** issue.
The dominant vulnerability class by far was **endpoints reachable without
authentication or without the authorization check their data warranted** —
six of the six Critical findings were exactly this shape: a route registered
outside the JWT-auth group, a permission bypass, or a webhook with no
signature check. This pattern was closed systematically by enumerating and
triaging **every** public route in the application (34 at last count), not
just the ones found by accident.

The one finding that remains open by deliberate choice — a payment-amount
trust issue in the fee-collection service — is not a gap in effort; it is a
decision not to guess at partial-payment/installment business rules with real
money at stake without the payments team's sign-off. It is fully specified in
`doc/security/payment-integrity-finding.md` with exact code pointers,
a reproducible exploit, and confirmed real-world impact.

Every fix in this report has been **verified against the current committed
code** (not just the commit that introduced it) — see §5.

---

## 2. Methodology

1. **Reconnaissance** — repository structure, dependency manifests, deployment
   pipeline, and stack identification (Laravel 12, JWT via `tymon/jwt-auth` +
   `firebase/php-jwt`, Next.js 16, Firebase, AWS S3, multiple third-party
   integrations: Facebook/TikTok lead ads, WhatsApp Business, Zoom, Genie and
   MyFees payment gateways, Gmail/Microsoft 365 email).
2. **Dependency & supply-chain review** — `composer.json`/`.lock`,
   `package.json`/`.lock`, CI/CD workflow.
3. **Full public-route enumeration** — every route reachable without
   authentication middleware was extracted programmatically and triaged one
   by one, rather than relying on manual discovery.
4. **Injection sink sweep** — every raw SQL query, file upload, file
   read/serve, HTML-rendering sink, and server-side outbound HTTP call in both
   codebases.
5. **Authentication & session review** — JWT issuance, verification,
   revocation, refresh, and every login-adjacent flow (password reset,
   entrance-exam login, OTP).
6. **Authorization / IDOR review** — RBAC middleware, per-request ownership
   checks in the student portal and admin controllers, mass-assignment
   exposure.
7. **Business-logic review** — payment creation and webhook handling,
   discount calculation.
8. **Remediation** — each confirmed finding fixed at the point of root cause,
   with the safest available mechanism (see the "Remediation approach" note
   in §4 for the pattern used repeatedly: fail-closed once configured,
   fail-open-with-a-loud-log otherwise, so fixes never silently break a
   working integration in production).
9. **Verification** — PHP lint on every changed file, `php artisan` boot
   test, full route-list diff, TypeScript `tsc --noEmit` on the frontend,
   and targeted functional tests (via `tinker` / Node) for every piece of new
   security logic (signature verification, masking, rate limiting, path
   validation, SSRF guards) — see §5 for the full verification run.

---

## 3. Findings summary

| # | ID | Title | Severity | OWASP category | Status |
|---|---|---|---|---|---|
| 1 | AUTHZ-1 | Unauthenticated full student record dump | **Critical** | A01 Broken Access Control | ✅ Fixed · residual sweep owed |
| 2 | AUTHZ-3 | Any logged-in student could call any admin endpoint | **Critical** | A01 Broken Access Control | ✅ Fixed |
| 3 | AUTHZ-7 | Unauthenticated media proxy — arbitrary file read + XSS | **Critical** | A01 / A03 | ✅ Hardened · signed-URL migration ready, opt-in |
| 4 | DATA-6 | Firestore reads from the browser were fully anonymous | **Critical** | A01 / A07 | ✅ Fixed in code · Firebase console + rules deploy owed |
| 5 | DATA-8 | DB backups on a web-served disk, no auth download path | **Critical** | A01 / A05 Security Misconfiguration | ✅ Fixed |
| 6 | API-9b | Payment: unauthenticated order-create + unsigned webhook + amount trust | **Critical** | A01 / A08 Data Integrity Failures | ◑ Partially fixed — root cause handed off (see dedicated doc) |
| 7 | AUTH-2 | No rate limiting anywhere | High | A07 Auth Failures | ✅ Fixed |
| 8 | AUTH-3 | Logout did not invalidate the token; no refresh rotation | High | A07 Auth Failures | ✅ Fixed |
| 9 | AUTH-5 | Logout revocation ran on a shadowed, dead middleware | High | A07 Auth Failures | ✅ Fixed |
| 10 | AUTH-7 | Password-reset lookup leaked full contact PII; no OTP lock | High | A07 / A01 | ✅ Fixed |
| 11 | AUTHZ-5 | Mass assignment — approval-field bypass | High | A08 Data Integrity Failures | ◑ Core bypass fixed · full permission audit owed |
| 12 | AUTHZ-8 | Public identifier lookup leaked NIC/passport/DOB | High | A01 / A02 | ✅ Fixed |
| 13 | API-8 | Error handling & `APP_DEBUG` defaults | High | A05 Security Misconfiguration | ✅ Fixed |
| 14 | API-9 | Webhooks accepted unsigned/unauthenticated requests | High | A07 / A08 | ✅ Fixed |
| 15 | CFG-1 | No security headers or CSP | High | A05 Security Misconfiguration | ◑ Fixed, CSP in Report-Only pending enforcement |
| 16 | CFG-4 | `.git` shipped to every production server | High | A05 Security Misconfiguration | ◑ Fixed in app; docroot verification owed |
| 17 | DATA-3 | Public settings endpoint ignored `is_public` | High | A01 / A02 Cryptographic/Data Exposure | ✅ Fixed |
| 18 | DEPS-3 | `xlsx@0.18.5` — prototype pollution + ReDoS | High | A06 Vulnerable Components | ✅ Fixed |
| 19 | INJ-1 | SQL injection | High | A03 Injection | ✅ Verified clean (no change needed) |
| 20 | INJ-2 | Stored XSS via unsanitised rich text | High | A03 Injection | ✅ Fixed |
| 21 | INJ-3 | File uploads — extension-only check, one path unrestricted | High | A03 / A08 | ✅ Fixed |
| 22 | SRC-8 | Lockfiles + CORS config were git-ignored | High | A05 / A08 | ✅ Fixed |
| 23 | API-1 | Dead diagnostic route file | Medium | A05 Security Misconfiguration | ✅ Fixed (removed) |
| 24 | AUTH-1b | Entrance-exam login was single-factor | Medium | A07 Auth Failures | ✅ Fixed |
| 25 | AUTHZ-4 | Permission middleware leaked the RBAC map + wildcard CORS | Medium | A05 / A01 | ✅ Fixed |
| 26 | DEPS-4 | Wildcard constraints + `minimum-stability: dev` | Medium | A06 Vulnerable Components | ◑ Fixed in composer.json; `firebase/php-jwt` update owed |
| 27 | DEPS-6 | CI/CD workflow hardening | Medium | A08 Supply Chain | ◑ Fixed; Action SHA-pinning owed |
| 28 | INJ-4 | SSRF via server-side fetchers + runtime `.env` writes | Medium | A10 SSRF | ✅ Fixed |
| 29 | INJ-5 | DomPDF forced remote access on | Medium | A10 SSRF | ✅ Fixed |
| 30 | INJ-6 | Spreadsheet / CSV formula injection on exports | Medium | A03 Injection | ✅ Fixed |
| 31 | SRC-14 | License declared MIT on a proprietary product | Low | — (IP/legal) | ✅ Fixed |
| 32 | SRC-7 | Real values in committed `.env.example` | Info | A05 Security Misconfiguration | ✅ Fixed |

**Totals: 6 Critical · 16 High · 8 Medium · 1 Low · 1 Info — 23 fully fixed, 9 fixed-with-follow-up, 0 unaddressed.**

---

## 4. Detailed findings

> Remediation approach used throughout: where a fix could plausibly break a
> currently-working third-party integration if the operator hasn't configured
> a required secret yet (webhooks, biometric device, payment gateways), the
> fix **fails closed once the secret/flag is configured, and fails open with
> a `CRITICAL`-level log line otherwise** — so nothing breaks silently in
> production, but the gap is loud and actionable in the logs until closed.

### 4.1 Critical

**AUTHZ-1 — Unauthenticated full student record dump**
`GET /api/v1/student/{studentId}` was registered *outside* the JWT-auth
group and routed to the admin `UserController::getStudent`, which returns the
complete profile — enrolments, documents, admissions, discounts, and the
**last 10 financial transactions** — with only an "id exists" check. A
duplicate assignment-submission route had the same defect.
*Fix:* both routes moved inside `jwt.auth`; an ownership guard added so a
student account may only read its own record (staff/admin unaffected).
*Residual:* `RoutePermissionMiddleware` still exempts the student role
entirely on `/admin/*`-style checks it doesn't cover, so each student-facing
controller must independently enforce ownership. A second pass
(`AcademicExamController::submitExam`) was found and fixed the same way; a
full sweep of the remaining student-portal controllers is the residual item.

**AUTHZ-3 — Any logged-in student could call any admin endpoint**
`RoutePermissionMiddleware` (the RBAC gate on `/api/v1/admin/*`) contained an
unconditional `if ($user->student()->exists()) { return $next($request); }`
— a blanket bypass. Any authenticated student JWT could reach any admin
route, read or write, limited only by what the controller itself happened to
check.
*Fix:* bypass removed. Students now receive 403 on `/admin/*` except a
narrow, unit-tested allow-list of non-PII reference reads and their own
attendance rows. A configurable escape hatch (`PERMISSIONS_ALLOW_STUDENT_READ`,
default `false`) exists for migration only. New scoped
`/api/v1/student/attendance/*` routes replaced the admin-route dependency the
student portal's self-check-in feature had relied on, so no functionality was
lost.

**AUTHZ-7 — Unauthenticated media proxy: arbitrary file read + XSS**
`GET /api/v1/media/proxy` took `path` and `disk` straight from the query
string with **no authentication, no path validation, and a disk default of
the private filesystem**. Any file on any disk was readable by URL, and the
endpoint's own extension→MIME map served `.html`/`.svg`/`.js` inline on the
API origin (reflected XSS), plus a folder-zip download for bulk exfiltration.
*Fix:* disk allow-listed to `public`/`s3`/`r2` (never private); path validated
against traversal, absolute paths, and wrappers; inline responses restricted
to a fixed image/PDF/audio/video MIME set with every other type forced to
`application/octet-stream` download.
*Follow-up shipped after the initial fix:* full signed-URL support
(`URL::temporarySignedRoute`) built and verified, gated behind
`MEDIA_PROXY_SIGNED_URLS` (default `false`) until the small number of
frontend components that build this URL by hand are migrated to a
backend-supplied link — see `deployment/README.md §5`.

**DATA-6 — Firestore reads from the browser were fully anonymous**
The frontend embeds the Firebase Web SDK with **no Firebase Authentication**
configured, so every Firestore read (per-user notifications, a real-time
attendance display) executed as an anonymous request — the security rules,
if any existed, were the only barrier, and the project id ships in the client
bundle.
*Fix:* a Firebase custom-token bridge — the API mints a token on login/refresh
(`uid = "user_<id>"`), the frontend signs in with it, and a
version-controlled `firestore.rules` restricts each notification path to its
owner and denies everything else by default.
*Residual:* the rules file must be deployed (`firebase deploy`) and a
sign-in method enabled in the Firebase console per client project — this is
an infrastructure action, not code.

**DATA-8 — Database backups on a web-served disk, no authenticated download**
Full SQL dumps (every row, including password hashes) were written to
`config('filesystems.default')` — if that resolved to a public disk, the dump
was web-reachable, and the (then-unauthenticated) media proxy made that a
one-request database exfiltration. The frontend also downloaded backups via a
bare, tokenless link.
*Fix:* backups pinned to the private disk unconditionally; a new
authenticated `GET /admin/backups/{fileName}/download` streams the file with
strict filename validation; the frontend now fetches it as an authenticated
blob.

**API-9b — Payment: unauthenticated order-create + unsigned webhook + amount trust**
See `doc/security/payment-integrity-finding.md` for the full write-up. In
summary: `POST /payments/{genie,myfees}/create` had no authentication; the
MyFees "mark order paid" webhook had no signature check at all; and neither
webhook verified the amount actually paid against the order amount.
*Fixed:* both create routes now require `jwt.auth`; the MyFees webhook is
gated on a shared secret; both webhooks reject a short payment
(`status = amount_mismatch`); a pre-existing but under-used authoritative
discount calculator is now invoked unconditionally instead of trusting a
client-supplied discount figure.
*Not fixed, and why:* the deeper issue — each invoice item's owed amount is
taken from client input rather than reconciled against the real fee-plan
amount and prior payments — was traced and confirmed exploitable (a
falsely-settled item is excluded from every outstanding-balance view), but
fixing it correctly requires installment/proration business rules this
review does not have authoritative access to. Implementing a guess here risks
breaking real fee collection; it is handed off as a fully-specified, standalone
document instead.

### 4.2 High

| ID | Title | Fix |
|---|---|---|
| AUTH-2 | No rate limiting anywhere | Named limiters added (`auth`: 5/min per identifier+IP; `otp`: 3/min + 15/hr; `api`: 300/min) and applied to every credential-adjacent route. |
| AUTH-3 | Logout didn't invalidate tokens; no refresh rotation | `TokenGuard` cache-backed denylist; refresh token rotates on use with reuse detection. |
| AUTH-5 | Revocation logic lived on a shadowed middleware | Discovered `jwt.auth` actually resolves to a third-party middleware, not the app's own — the revocation check was dead code. A new `CheckTokenRevocation` middleware, independent of which auth middleware wins, enforces it on the real request path. |
| AUTH-7 | Password-reset lookup leaked full PII; no OTP lockout | Response masks email/phone, drops name/NIC/DOB entirely; OTP burns after 5 wrong guesses. |
| AUTHZ-5 | Mass assignment — approval-field bypass | `approved_by`/`approved_at`/`received_by` were accepted from the client on 3 finance controllers (inventory, purchase orders, discounts) and could self-assert an approval while paying a token amount. Now stripped server-side on every store/update; approval only via the dedicated approve action. |
| AUTHZ-8 | Public identifier lookup leaked NIC/passport/DOB | Response masked to id + masked name/email/phone; sensitive fields dropped; endpoint rate-limited. |
| API-8 | Error handling & `APP_DEBUG` | JSON error responses forced on every `/api/*` path regardless of `Accept` header; `.env.example` given explicit production-safe defaults. |
| API-9 | Webhooks accepted unsigned requests | Facebook/TikTok's "verify only if a signature is present" logic (silently skippable) fixed to require it; WhatsApp, Fringer (biometric device) and Zoom gained signature/shared-secret checks where none existed. |
| CFG-1 | No security headers or CSP | Full header set on both apps (HSTS, `X-Frame-Options`, `nosniff`, `Referrer-Policy`, `Permissions-Policy`); CSP shipped in Report-Only mode pending an enforce decision. |
| CFG-4 | `.git` shipped to every server | Root + `public/` `.htaccess` deny-all safety nets, an Nginx hardening snippet, and a deploy-verification runbook. |
| DATA-3 | Public settings endpoint ignored `is_public` | Now filters `is_public = true` at the query level plus a key-name denylist (`secret`, `password`, `token`, `_json`, …) as defence in depth. |
| DEPS-3 | `xlsx@0.18.5` CVEs | Repointed to the maintained SheetJS build; added a hardened parse wrapper on both upload sites. |
| INJ-1 | SQL injection | Swept ~150 raw-query sites — all parameterised. No change required; documented as verified. |
| INJ-2 | Stored XSS | `isomorphic-dompurify` + a shared `sanitizeHtml()` applied to all 24 `dangerouslySetInnerHTML` sinks, including student-submission text rendered in a staff/grader session. |
| INJ-3 | File upload validation | A real-bytes MIME check added to the shared upload trait; one controller that bypassed it entirely (accepting any file type onto a public disk) locked to a safe extension list. |
| SRC-8 | Lockfiles + CORS config git-ignored | Un-ignored and committed; the actual CORS enforcement (a custom middleware that had hard-coded `Access-Control-Allow-Origin: *`) rewritten to an environment-driven allow-list. |

### 4.3 Medium

| ID | Title | Fix |
|---|---|---|
| API-1 | Dead diagnostic route file | Deleted (leaked server config/PHP version if ever wired up; confirmed unreachable). |
| AUTH-1b | Single-factor entrance-exam login | Date-of-birth added as a required second factor. |
| AUTHZ-4 | Permission middleware leaked the RBAC map | Debug payload gated behind `config('app.debug')`; a hard-coded wildcard CORS header on 403 responses removed. |
| DEPS-4 | Wildcard version constraints, `minimum-stability: dev` | Wildcards replaced with bounded ranges; stability set to `stable`; lockfile hash recomputed and verified against Composer's own algorithm. |
| DEPS-6 | CI/CD hardening | Least-privilege `GITHUB_TOKEN`, a concurrency guard, push-only production deploys (a pull_request could previously trigger one), and the test/audit steps made visible instead of silently swallowed. |
| INJ-4 | SSRF + runtime `.env` writes | A public-IP-only guard added to a server-side URL fetch (blocks cloud metadata/loopback/private ranges); a feature that wrote to `.env` on every request migrated to the settings table. |
| INJ-5 | DomPDF remote access | `isRemoteEnabled`/`enable_remote` forced off; `chroot` narrowed from the whole app to `public/`. |
| INJ-6 | Spreadsheet/CSV formula injection | A value binder neutralises formula-triggering cell content on export (xlsx and CSV) without altering legitimate numeric/text data. |

### 4.4 Low / Informational

- **SRC-14** — both projects declared `MIT` despite being proprietary; corrected to `proprietary`/`UNLICENSED`.
- **SRC-7** — the frontend's committed `.env.example` carried real vendor contact details; replaced with placeholders.

---

## 5. Verification performed

Every item above was checked against the code **currently in the repository**
(not assumed from the commit that introduced it):

- **Backend:** 45 targeted checks (one or more per finding) run against the
  live file contents; every changed PHP file linted individually; a full
  `php artisan` boot test; a `route:list` diff confirming no route regressed.
- **Frontend:** 13 targeted checks; `tsc --noEmit` across the full project —
  clean.
- **Functional tests** run directly (via `tinker` / Node) for every new
  security primitive rather than relying on code review alone, for example:
  webhook signature verification against forged and tampered payloads, the
  path-traversal guard against 8 attack strings, the SSRF guard against cloud
  metadata/loopback/private-range URLs, PII-masking output, the OTP
  attempt-lockout counter, and the signed-media-URL signature surviving/
  rejecting tampering.

Result: **0 regressions, 0 lint failures, 0 TypeScript errors** attributable
to this work.

---

## 6. Outstanding items (owed, not overlooked)

| Item | Why it's not closed in code |
|---|---|
| Payment amount-trust root cause (API-9b) | Needs payments-team domain sign-off — see dedicated document. |
| Firestore rules deployment (DATA-6) | Requires Firebase console access, one action per client project. |
| CSP enforcement (CFG-1) | Currently Report-Only by design — needs a period of monitoring before flipping to blocking. |
| Docroot verification (CFG-4) | Requires access to each production vhost config. |
| `firebase/php-jwt` version bump (DEPS-4) | Requires `composer update` on a machine with the `openssl` PHP extension (unavailable in this environment). |
| GitHub Actions SHA-pinning (DEPS-6) | Deliberately not guessed — a wrong SHA silently breaks the deploy pipeline; exact resolution command left in the workflow file. |
| Signed media URLs (AUTHZ-7) | Built and tested; withheld from default-on until ~4 frontend components are migrated off hand-built proxy URLs (listed in `deployment/README.md`). |
| Mass-assignment / permission full sweep (AUTHZ-5) | The concrete exploit found is fixed; a full audit of ~200 admin routes' assigned permissions needs a running app + test roles. |
| Student-portal ownership full sweep (AUTHZ-1) | Spot-checked and one real gap fixed; a complete pass needs the same. |

None of these are silent gaps — each is logged, documented, and either
config-gated or specified precisely enough for someone with the missing
context (server access, Firebase console, payments domain knowledge) to
close it without guessing.

---

## 7. Recommendations going forward

1. Commit the staged remediation (backend + frontend) and work the
   "Outstanding items" table above against a staging environment before
   production.
2. Route `doc/security/payment-integrity-finding.md` to whoever owns the
   payments/billing module as the next priority — it is the only Critical
   still open.
3. Pair this report with `doc/security/server-hardening-guide.md` — a
   hardened application on an unhardened server is still compromised the
   moment the server is.
4. Re-run a full review (`/code-review ultra` or equivalent) quarterly, and
   after any significant new feature that adds a public route, a webhook, or
   handles payment/PII data — those three categories accounted for the large
   majority of findings here.
5. Adopt the pattern used throughout this remediation for any new
   third-party integration: **require its signature/secret, fail closed once
   configured, log loudly if not** — it is what made every webhook fix safe
   to ship without breaking a live integration.