1b66e0b6ff
First full scan (no docs/security-kb.md yet). 2 High, 3 Medium, 3 Low, 4 Informational findings — most notable: national_id stored/exported in plaintext despite architecture.md claiming encryption, and a confirmed CVE (CVE-2026-48019, CVSS 8.9) affecting the installed laravel/framework 12.43.1 via the guest-booking form's Mail::to() call. Report only — no fixes applied, per the security-scan skill's non-capabilities. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
112 lines
17 KiB
Markdown
112 lines
17 KiB
Markdown
# Security Report
|
||
|
||
**Project:** Libra Law Firm
|
||
**Scan date:** 2026-09-27
|
||
**Scope:** full codebase
|
||
|
||
---
|
||
|
||
## Summary
|
||
|
||
This is a static-analysis-only review of `app/`, `resources/views/livewire/`, `routes/`, `config/`, `database/migrations/`, and `bootstrap/` (excluding `vendor/`, `node_modules/`, `storage/`, `.git/`, `docs/`, `.bmad-core/`). The scan followed the priority order below. The most significant finding is that `national_id` — despite `docs/architecture.md` §13.4 documenting it as encrypted-at-rest — is stored, displayed, exported and audit-logged in **plaintext** throughout the codebase; no encrypted cast exists on the `User` model. No SQL injection was found in the reviewed files (Eloquent/query-builder used consistently with parameter binding). No findings at Critical severity in the files reviewed.
|
||
|
||
## Scan order followed
|
||
|
||
1. Authentication & session handling, input validation, injection
|
||
2. Authorization & access control (admin/client role checks, IDOR)
|
||
3. Sensitive data exposure & cryptography
|
||
4. Remaining checklist: XSS, mass assignment, dependencies, misconfiguration, CORS, headers
|
||
5. Guest-facing surfaces (booking form, `CaptchaService`)
|
||
|
||
## Findings
|
||
|
||
### [HIGH] — `national_id` stored, displayed, exported, and audit-logged in plaintext, contradicting documented encryption
|
||
- **File:** `app/Models/User.php:24-47` (fillable/hidden, no cast); `database/migrations/0001_01_01_000000_create_users_table.php:18` (`$table->string('national_id')`); `resources/views/livewire/admin/clients/individual/show.blade.php:64`; `resources/views/livewire/admin/clients/individual/index.blade.php:145`; `resources/views/livewire/admin/users/export-users.blade.php:52`; `resources/views/livewire/admin/clients/individual/edit.blade.php:57,78`; `resources/views/livewire/admin/clients/individual/create.blade.php:61`
|
||
- **Severity:** High
|
||
- **Confidence:** High
|
||
- **Description:** `docs/architecture.md` §13.4 documents an `Attribute::make(get: decrypt, set: encrypt)` cast for `national_id` on `User`. No such cast exists in the actual model — `national_id` is a plain `string` column, mass-assignable, and only excluded from array/JSON serialization via `$hidden` (which doesn't protect direct DB access, backups, or explicit `->only([...])` reads). It is displayed in cleartext in the admin client show/index views, included in CSV user exports, and copied verbatim into `AdminLog.old_values`/`new_values` on every create/update (see next finding) — multiplying the exposure surface for a national ID number (sensitive PII for a law firm's clients) well beyond what the architecture doc claims is protected.
|
||
- **Recommendation:** Either implement the encrypted-attribute cast the architecture document already specifies (Laravel's `encrypted` cast type is a one-line change: `'national_id' => 'encrypted'`), or update `docs/architecture.md` to reflect actual practice and add compensating controls (restrict export/display, exclude from audit-log payloads). Decide deliberately — don't leave the documentation and code disagreeing.
|
||
|
||
### [HIGH] — Guest email address user-supplied to `Mail::to()` on a Laravel version vulnerable to CVE-2026-48019 (CRLF injection in email validation)
|
||
- **File:** `resources/views/livewire/pages/booking.blade.php:78,168` (`'guestEmail' => ['required','email',...]` then `Mail::to($this->guestEmail)->queue(...)`); `composer.lock` — `laravel/framework` pinned at `v12.43.1`
|
||
- **Severity:** High
|
||
- **Confidence:** High
|
||
- **Description:** Confirmed via WebSearch: GHSA-5vg9-5847-vvmq / CVE-2026-48019 (CVSS 8.9) is a CRLF injection vulnerability in Laravel's default `email` validation rule, in combination with how Symfony Mailer/Mime handle certain byte sequences, affecting all `laravel/framework` versions below `12.60.0` (fixed in 12.60.0 and 13.10.0). This app's installed version is `12.43.1`, inside the vulnerable range, and the unauthenticated guest booking form passes a user-supplied email address (validated only with Laravel's stock `email` rule) directly into `Mail::to()`. Per the advisory, an unauthenticated attacker may be able to interfere with outbound mail content/recipients under this condition.
|
||
- **Recommendation:** Upgrade `laravel/framework` to `^12.60` (or `^13.10` if a major upgrade is in scope) — do not implement a custom mitigation in place of the vendor patch. No `docs/security-kb.md` exists yet to track this class of issue; recommend creating one so future dependency bumps get checked against known advisories systematically rather than ad hoc.
|
||
|
||
### [MEDIUM] — Timeline "initial note" stored without HTML sanitization, then rendered unescaped to both admin and client
|
||
- **File:** `resources/views/livewire/admin/timelines/create.blade.php:87` (`'update_text' => $this->initialNotes` — no `clean()`); rendered raw at `resources/views/livewire/client/timelines/show.blade.php:49`, `resources/views/livewire/admin/timelines/show.blade.php:311`, `resources/views/emails/timeline-update.blade.php:19,44`, `resources/views/emails/timeline/update/{ar,en}.blade.php:17`
|
||
- **Severity:** Medium
|
||
- **Confidence:** High
|
||
- **Description:** Every other `update_text` write path in the codebase runs the value through `clean()` (HTMLPurifier via `mews/purifier`) before saving — see `resources/views/livewire/admin/timelines/show.blade.php:46,84`. The *initial* note created alongside a new timeline (`admin/timelines/create.blade.php`) skips this and stores raw input, which is then output with `{!! !!}` (unescaped) in the client-facing timeline view, the admin timeline view, and outbound emails. Admin is currently the single trusted operator, so this isn't exploitable by an outside attacker directly, but it is an inconsistency in a defense-in-depth control: if the admin's account or session were ever compromised (credential phishing, XSS elsewhere, session hijack — 2FA is currently disabled for this account per `config/fortify.php`, raising the value of protecting this account further), this is the one write path that would let injected markup/script persist and execute in the client's authenticated session, not just the admin's own.
|
||
- **Recommendation:** Route `initialNotes` through the same `clean()` call used everywhere else `update_text` is written, for consistency with the rest of the codebase's own sanitization pattern.
|
||
|
||
### [MEDIUM] — `email_verified_at` / `verified` middleware not enforced on any protected route
|
||
- **File:** `routes/web.php` (admin/client/settings groups use `Route::middleware(['auth', 'active'])...` — no `verified`); `docs/architecture.md` §7.1/§9.2 documents `auth, verified, admin` / `auth, verified, active` as the intended stack
|
||
- **Severity:** Medium
|
||
- **Confidence:** Medium
|
||
- **Description:** Fortify's `emailVerification()` feature is enabled (`config/fortify.php:149`), implying email ownership is meant to gate access, and the architecture doc explicitly lists `verified` in the admin and client middleware stacks. The actual `routes/web.php` never applies Laravel's `verified` middleware anywhere — only `auth` and `active`. Since accounts are admin-created (no public registration) impact is reduced, but it means a client account created with a mistyped or attacker-influenced email address is fully usable (dashboard, timelines, consultations) without ever confirming that mailbox, and the documented control isn't actually the one in force. Judgment call on whether this was an intentional simplification or a regression belongs to whoever owns this project's requirements, not to this scan.
|
||
- **Recommendation:** Either add `verified` back into the `admin`/`client`/`settings` middleware groups to match the documented design, or update `docs/architecture.md` to state that verification is informational-only for this app and explain why.
|
||
|
||
### [MEDIUM] — Weak, low-entropy guest-booking captcha combined with per-IP-only rate limiting
|
||
- **File:** `app/Services/CaptchaService.php:14-26` (single-digit-sum math question, session-stored); `resources/views/livewire/pages/booking.blade.php:119-126` (`RateLimiter` keyed only on `request()->ip()`, 5/24h)
|
||
- **Severity:** Medium
|
||
- **Confidence:** Medium
|
||
- **Description:** The captcha answer space is tiny (sum of two numbers 1–10, i.e. 2–20, ~19 possible answers) with no image/behavioral challenge, so a scripted client can solve it by brute force without needing OCR — it primarily filters unsophisticated bots, not a determined scripted one. The only additional control is a rate limit keyed solely on `request()->ip()`, which is trivially defeated by rotating source IPs/proxies (common for scripted abuse), and the 1-per-email/day check (`booking.blade.php:92-102`) only limits repeats from the *same* guest email, not overall volume. Together these leave the public, unauthenticated booking form exposed to scripted spam/abuse of the booking calendar and of the guest's own inbox (see next finding).
|
||
- **Recommendation:** Consider a higher-entropy or interaction-based challenge (e.g., a proper CAPTCHA/Turnstile) for the public booking form, and/or a global (not just per-email) submission rate limit for guest bookings, independent of the per-IP limiter.
|
||
|
||
### [LOW] — Guest booking form sends a "confirmation" email to an arbitrary, unauthenticated-supplied address
|
||
- **File:** `resources/views/livewire/pages/booking.blade.php:167-170` (`Mail::to($this->guestEmail)->queue(new GuestBookingSubmittedMail($consultation))`)
|
||
- **Severity:** Low
|
||
- **Confidence:** Medium
|
||
- **Description:** `guestEmail` is only validated as a well-formed email address (`'guestEmail' => ['required','email','max:255']`) — it is never confirmed to belong to the submitter. Combined with the weak captcha/rate-limiting above, this lets an attacker cause automated emails (containing a booking confirmation, and the submitted `problemSummary` text) to be delivered to a third party's inbox of the attacker's choosing, at low but nonzero volume (5 submissions per IP per 24h). This is a mail-relay/nuisance vector, not data exposure of an existing account.
|
||
- **Recommendation:** Low priority; if this becomes an actual abuse vector in practice, consider requiring email confirmation before dispatching the guest-facing notification, independent of the admin notification.
|
||
|
||
### [LOW] — No security-headers middleware present despite being documented
|
||
- **File:** `app/Http/Middleware/` (only `SetLocale`, `EnsureUserIsAdmin`, `EnsureUserIsClient`, `EnsureUserIsActive` exist); `bootstrap/app.php:16-24`; contrast with `docs/architecture.md` §13.3 (`ContentSecurityPolicy` middleware, CSP/X-Frame-Options/X-Content-Type-Options/Referrer-Policy)
|
||
- **Severity:** Low
|
||
- **Confidence:** High
|
||
- **Description:** The architecture document specifies a `ContentSecurityPolicy` middleware setting CSP, `X-Content-Type-Options`, `X-Frame-Options`, `X-XSS-Protection`, and `Referrer-Policy` headers. No such middleware exists anywhere in `app/Http/Middleware/`, and `bootstrap/app.php`'s middleware registration doesn't reference one. The app currently ships with none of Laravel's non-default security headers set, relying only on framework/browser defaults.
|
||
- **Recommendation:** Implement the documented headers middleware (or adopt a maintained package for it) if this level of hardening is still wanted; otherwise correct the architecture doc.
|
||
|
||
### [LOW] — `.env.example` omits `SESSION_SECURE_COOKIE`, no `config/cors.php` published
|
||
- **File:** `.env.example` (no `SESSION_SECURE_COOKIE` key at all); `config/` (no `cors.php` file)
|
||
- **Severity:** Low
|
||
- **Confidence:** Medium
|
||
- **Description:** `docs/architecture.md` §13.1 lists `SESSION_SECURE_COOKIE=true` as a documented security measure, but `.env.example` — the template a deployer copies to build a production `.env` — has no `SESSION_SECURE_COOKIE` line at all, so `config/session.php:172`'s `env('SESSION_SECURE_COOKIE')` silently resolves to `null`/falsy unless someone remembers to add it by hand at deploy time. Separately, no `config/cors.php` is published; since this app has no `/api/*` routes, the practical exposure is minimal (Laravel's built-in `HandleCors` default only applies to `api/*` by default), but its absence should be a deliberate choice, not an oversight, if that ever changes.
|
||
- **Recommendation:** Add `SESSION_SECURE_COOKIE=true` to `.env.example` (and confirm the production `.env` on the actual server sets it — out of scope for this static scan, see below). Publish `config/cors.php` explicitly if any API-style routes are ever added.
|
||
|
||
### [INFORMATIONAL] — Local `.env` reviewed, no live secrets found; not a production file
|
||
- **File:** `.env`, `.gitignore:1-3`
|
||
- **Severity:** Informational
|
||
- **Confidence:** High
|
||
- **Description:** `.env` is correctly excluded from git (`.gitignore` lists `.env`, `.env.backup`, `.env.production`). Its contents are local-development values only (`APP_ENV=local`, `APP_DEBUG=true`, root DB user with no password, localhost mail catcher, no AWS/mail credentials populated) — no live secrets are present in this repository checkout. `APP_DEBUG=true` is appropriate for local but must not carry into production; this scan cannot confirm the production server's actual `.env` (out of scope, see below).
|
||
- **Recommendation:** None needed for this file as reviewed. Confirm separately (via `infra-ops`/production access, not this scan) that the production `.env` sets `APP_DEBUG=false`.
|
||
|
||
### [INFORMATIONAL] — IDOR checks manually verified as present on key client-facing record views
|
||
- **File:** `resources/views/livewire/client/timelines/show.blade.php:10-13` (`abort_unless($timeline->user_id === auth()->id(), 403)`); `routes/web.php` client consultation-calendar closure (`abort_unless($consultation->user_id === auth()->id(), 403)`); `resources/views/livewire/client/consultations/index.blade.php:15-43` (scoped to `auth()->user()->consultations()`)
|
||
- **Severity:** Informational
|
||
- **Confidence:** High
|
||
- **Description:** Checked specifically for the scenario the task called out — a client viewing another client's consultation/timeline by guessing an ID. All three client-facing record-access paths reviewed do explicitly scope to the authenticated user, either via an `abort_unless` ownership check on route-model-bound records or by querying through the `auth()->user()->consultations()` relation rather than the bare model. No IDOR found in these paths.
|
||
- **Recommendation:** None. Noting this as a confirmed-safe pattern so it isn't re-flagged as a gap by a future scan without re-checking.
|
||
|
||
### [INFORMATIONAL] — XSS output-encoding spot checks passed on other rich-text paths
|
||
- **File:** `resources/views/livewire/pages/posts/index.blade.php:25-39` (`highlightSearch()` escapes both text and search term via `e()` before wrapping in `<mark>`); `resources/views/livewire/admin/posts/{create,edit}.blade.php`, `resources/views/livewire/admin/pages/edit.blade.php`, `resources/views/pages/legal.blade.php:6` (all route admin-authored rich text through `clean()` / HTMLPurifier before storage or display)
|
||
- **Severity:** Informational
|
||
- **Confidence:** High
|
||
- **Description:** All other `{!! !!}` (unescaped) output sites reviewed pair the raw output with sanitization at write time (`clean()`) or with pre-escaping (`e()`) before HTML-wrapping, except for the timeline `initial notes` path already flagged above.
|
||
- **Recommendation:** None beyond the Medium finding already listed.
|
||
|
||
---
|
||
|
||
## What was NOT checked
|
||
|
||
- **Runtime behavior.** Nothing in this codebase was executed, run, or tested; all findings are static code review only.
|
||
- **The live production server / hosting configuration** (69.62.106.94, PHP-FPM config, actual production `.env`, TLS/HTTPS enforcement, firewall, MariaDB hardening). Confirming `APP_DEBUG=false` and `SESSION_SECURE_COOKIE=true` on the real production `.env` is explicitly out of scope here and was not verified.
|
||
- **Third-party API/service behavior** — the SMTP provider, any queue worker/Supervisor process behavior, actual mail deliverability.
|
||
- **Infrastructure-as-code / deployment scripts** referenced in `docs/architecture.md` §16 (CI pipeline, `deploy.sh`, `rollback.sh`) were not audited as part of this application-code scan.
|
||
- **Excluded directories per scope:** `vendor/`, `node_modules/`, `storage/`, `.git/`, `docs/`, `.bmad-core/`.
|
||
- **Dependency/CVE coverage is spot-check only, not exhaustive.** No `docs/security-kb.md` exists yet for this project (no prior stack-aware CVE research has been done), so the dependency review above covered only the highest-traffic packages (`laravel/framework`, `mews/purifier`) via targeted WebSearch, not the full `composer.lock`/`package-lock.json` dependency tree. The one CVE cited (CVE-2026-48019 / GHSA-5vg9-5847-vvmq) was independently confirmed via search, not fabricated; no other CVE numbers are claimed in this report. A full dependency audit (building `docs/security-kb.md`) is recommended as separate follow-up work.
|
||
- **Frontend/JS dependencies** (`package.json`/`package-lock.json`, Tailwind 4/Vite build) were not cross-checked against advisories.
|
||
- **Business-logic abuse beyond IDOR/spam** (e.g., booking-slot race conditions beyond what's visible in the `DB::transaction`/`lockForUpdate` calls already in `booking.blade.php`) was not exhaustively modeled.
|