TS_Proto/security/VAPT-REPORT.md

# TravelHub — Whitebox Vulnerability Assessment & Penetration Test

**Engagement type:** Authorised whitebox (source-assisted) VAPT
**Target:** TravelHub multi-tenant flight-booking platform — Go API (`:8080`), Angular SPA (`:4200`), PostgreSQL 17
**Scope:** `backend/`, `frontend/`, database, and running instances on localhost
**Method:** Source review of the full codebase plus dynamic testing against the running stack
**Date:** 12 September 2026

---

## 1. Executive summary

TravelHub is architecturally sound on the issues that most often sink a
multi-tenant application. Tenant isolation holds under direct testing, SQL is
parameterised throughout, the JSON decoder rejects unknown fields, request
bodies are size-capped, and the Angular client escapes interpolated data by
default. The 311-test automated suite already guards the core business-logic
invariants (seat accounting, cross-tenant reads, overselling).

The weaknesses found are **operational and defence-in-depth**, not broken access
control. The two that matter most are a **database superuser connection** — which
turns any application compromise into total database control and nullifies the
carefully built application-layer tenant isolation — and a **timing side-channel
that backfired**: the mitigation intended to hide which usernames exist actually
makes them *more* distinguishable.

None of the findings is a remotely exploitable pre-authentication breach on the
current localhost deployment. Several become serious the moment this is exposed
beyond localhost, which the README states is the intended next step.

### Findings at a glance

| # | Severity | Finding |
| - | -------- | ------- |
| 1 | **High** | Application connects to PostgreSQL as a superuser (no least privilege) |
| 2 | **High** | Username enumeration via a timing oracle — the equalisation mitigation is miscalibrated and inverts |
| 3 | Medium | Bcrypt cost factor 6, below the OWASP-recommended minimum of 10 |
| 4 | Medium | No rate limiting or account lockout on authentication |
| 5 | Medium | Tenant enumeration via timing (unknown tenant short-circuits before password check) |
| 6 | Medium | No HTTP security headers (HSTS, X-Content-Type-Options, X-Frame-Options, CSP…) |
| 7 | Medium | Database connections run in plaintext (`sslmode=disable`) |
| 8 | Low | Real database password committed in `config_test.go` |
| 9 | Low | Credentials held reversibly in `localStorage` and replayed on every request (inherent to Basic auth) |
| 10 | Low | API stores markup verbatim — no server-side output encoding (defence-in-depth only) |
| 11 | Info | `RequireAdmin` middleware is implemented but wired to no route; the role model is unenforced |

---

## 2. Scope, method and rules of engagement

This was a **whitebox** test: the assessor had the full source tree, the
database, and running instances. Testing combined:

- **Static review** — grep-driven and manual reading of the auth middleware,
  repository (SQL) layer, HTTP handlers, configuration loader, and Angular auth
  service and interceptor.
- **Dynamic testing** — authenticated and unauthenticated requests against the
  live API: SQL-injection probes, IDOR attempts, mass-assignment, brute-force
  and timing measurement, stored-XSS sink analysis, payload-size and CORS tests.

Destructive tests (e.g. `DROP TABLE` injection) were run against the
development database with immediate cleanup. No third-party or production
systems were touched. All demo data was restored to its seeded state on
completion.

---

## 3. What was tested and found robust

These were actively attacked and **held**; they are recorded so the report is a
fair account, not only a list of faults.

| Control | Test | Result |
| ------- | ---- | ------ |
| SQL injection | `' OR '1'='1`, `'; DROP TABLE bookings;--`, `UNION SELECT` in `origin`/`destination` | Parameterised (`$1…$n`); payloads treated as literal IATA codes, `total=0` |
| Injection via `ORDER BY` | `sort=(SELECT 1)`, `sort=price;DROP` | `orderClause()` whitelists to three fixed clauses; junk falls back to price |
| Mass assignment | POST booking with `status`, `totalAmount`, `bookingReference`, `tenantId` | Rejected 400 — `DisallowUnknownFields()`; price/PNR/tenant are server-derived |
| Cross-tenant read (IDOR) | Fetch another agency's booking/flight by id | 404 `ErrNotFound`; every query filters `tenant_id` from the identity, not the request |
| Credential misuse across tenants | Skyline password against Globetrotter tenant | 401 |
| DoS via large body | 3 MB request body | 400 — `http.MaxBytesReader` caps at 1 MB |
| CORS | Reflect `Origin: https://evil.example.com` | Not reflected; only the configured allow-list origin receives ACAO + credentials |
| Error verbosity | Malformed UUID, malformed JSON | Generic messages; no stack traces or SQL leaked |
| Reflected XSS | Angular templating | No `innerHTML`/`bypassSecurityTrust`/`eval` anywhere; interpolation auto-escapes |

---

## 4. Detailed findings

### FINDING-1 — Application connects to PostgreSQL as a superuser  ·  **HIGH**

**Category:** CWE-250 Execution with Unnecessary Privileges · A04:2021 Insecure Design

**Evidence.** The application connects with the credentials in `backend/.env`
(`DB_USER=postgres`). That role is the cluster superuser:

```
 rolname  | rolsuper | rolcreatedb | rolcreaterole | rolbypassrls
----------+----------+-------------+---------------+--------------
 postgres | t        | t           | t             | t
```

**Impact.** Tenant isolation in TravelHub is enforced entirely in application
code — every repository query filters on `tenant_id`. That is a reasonable
design, but it means the database connection is the last line of defence, and
here it has *no* limits. Any flaw that yields query control (a future missed
`WHERE`, a dynamic-SQL regression, an ORM swap) is not a one-tenant leak but
full read/write/DDL over every tenant, plus `COPY … TO PROGRAM` command
execution and `rolbypassrls` defeating any future row-level security. The
superuser connection silently converts application bugs into total compromise.

**Recommendation.** Create a least-privilege role the app connects as:

```sql
CREATE ROLE travelapp LOGIN PASSWORD '…';
GRANT CONNECT ON DATABASE travelhub TO travelapp;
GRANT USAGE ON SCHEMA public TO travelapp;
GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA public TO travelapp;
-- deliberately NOT: SUPERUSER, CREATEROLE, CREATEDB, TRUNCATE, DDL
ALTER DEFAULT PRIVILEGES IN SCHEMA public
  GRANT SELECT, INSERT, UPDATE, DELETE ON TABLES TO travelapp;
```

Keep migrations under a separate, higher-privilege role that the running
service never uses.

---

### FINDING-2 — Username enumeration via an inverted timing oracle  ·  **HIGH**

**Category:** CWE-208 Observable Timing Discrepancy · CWE-203 Observable Discrepancy

**Evidence.** `middleware.go` deliberately performs a bcrypt comparison against a
dummy hash when a user is not found, to equalise response time (a correct
instinct). But the dummy hash is **cost 10**:

```go
var dummyHash = []byte("$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy")
```

while the real stored hashes are **cost 6** (`002_seed.sql` uses pgcrypto
`gen_salt('bf')`, whose default is 6). Because verifying a cost-10 hash costs
~16× a cost-6 hash, the "user does not exist" path is dramatically *slower* than
the "user exists, wrong password" path. Measured (mean of 30):

```
valid user + wrong password : 5.1 ms
unknown user, valid tenant  : 47.3 ms   ← ~9x slower
```

**Impact.** An unauthenticated attacker distinguishes valid from invalid
usernames with a single request each, at ~9× separation — a wide, reliable
margin needing no statistics. This enumerates the tenant's user directory
(valid email addresses) for targeted phishing or focused brute force. The
mitigation did not merely fail; it created a *stronger* oracle than doing
nothing, because the two branches now use different work factors.

**Recommendation.** Make the dummy hash's cost identical to the real one, and
pin both. After raising the real cost to 12 (see FINDING-3), regenerate the
dummy at cost 12 so both branches do equal work. Better still, verify against a
per-deployment constant hash generated at startup with the exact configured
cost, so the two can never drift apart again.

---

### FINDING-3 — Bcrypt cost factor below recommended minimum  ·  **MEDIUM**

**Category:** CWE-916 Use of Password Hash With Insufficient Computational Effort

**Evidence.** All stored hashes carry the `$2a$06$` prefix — cost 6:

```
admin@skyline.test | $2a$06$ | 60
agent@skyline.test | $2a$06$ | 60
```

`002_seed.sql` relies on pgcrypto's `gen_salt('bf')` default (6). The test
helper `testsupport.go` uses `bcrypt.MinCost` (4) — acceptable for test speed,
but it means production hashing strength is never exercised by the suite.

**Impact.** Cost 6 is 2⁴ = 16× weaker than the OWASP-recommended cost-10 floor
and 64× weaker than cost 12. Against an offline attacker who obtains the hashes
(see FINDING-1), it materially shortens the time to crack weak and medium
passwords.

**Recommendation.** Hash at **cost 12** in application code rather than in SQL,
so the work factor lives with the app and is consistent. Hash on the Go side
during any user-creation flow (`bcrypt.GenerateFromPassword(pw, 12)`); reseed
demo users accordingly. Support transparent rehash-on-login when an old-cost
hash is encountered.

---

### FINDING-4 — No rate limiting or account lockout on authentication  ·  **MEDIUM**

**Category:** CWE-307 Improper Restriction of Excessive Authentication Attempts · A07:2021

**Evidence.** 25 failed logins for a valid account completed in 144 ms, all
returning 401 — no `429`, no delay, no lockout:

```
25 failed logins in 144 ms -> 25x HTTP 401
```

**Impact.** Online password brute force and credential stuffing are unthrottled.
Combined with FINDING-2 (which hands the attacker the valid usernames) and
FINDING-3 (weaker hashes), the account-takeover path is realistic once exposed
beyond localhost.

**Recommendation.** Add per-source-IP and per-account throttling with
exponential backoff and temporary lockout after N failures. A middleware layer
(e.g. token-bucket keyed on IP + `X-Tenant` + email) fits the existing `Chain`
composition. Return `429` with `Retry-After`. Log repeated failures for
alerting.

---

### FINDING-5 — Tenant enumeration via timing  ·  **MEDIUM**

**Category:** CWE-204 Observable Response Discrepancy

**Evidence.** An unknown *tenant* short-circuits before any password check
(`TenantBySlug` returns `ErrNotFound`, and the handler returns before the bcrypt
call), so it is far faster than a known tenant with bad credentials:

```
unknown tenant             : 0.8 ms
valid tenant, wrong pw     : 5.1 ms
```

**Impact.** An attacker can discover which agency slugs exist. Lower impact than
FINDING-2 (the public `GET /api/tenants` endpoint already lists active tenants
by design), but it also reveals *suspended* tenants, which the public endpoint
deliberately hides.

**Recommendation.** Perform the dummy bcrypt comparison on the unknown-tenant
path too, so all failure modes cost the same. Fold this into the FINDING-2 fix.

---

### FINDING-6 — Missing HTTP security headers  ·  **MEDIUM**

**Category:** CWE-693 Protection Mechanism Failure · A05:2021 Security Misconfiguration

**Evidence.** API responses carry only `Content-Type`, `Content-Length`, `Date`.
Absent: `Strict-Transport-Security`, `X-Content-Type-Options: nosniff`,
`X-Frame-Options`/`frame-ancestors`, `Content-Security-Policy`,
`Referrer-Policy`, `Cache-Control` on authenticated JSON. The Angular
`index.html` ships no CSP either.

**Impact.** No MIME-sniffing protection, no clickjacking defence, no CSP to
blunt an XSS should one ever be introduced, and browsers may cache
authenticated responses. Individually minor; collectively they remove several
cheap layers of defence.

**Recommendation.** Add a `SecureHeaders` middleware to the `Chain`:
`X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`,
`Referrer-Policy: no-referrer`, `Cache-Control: no-store` on authenticated
responses, and `Strict-Transport-Security` once TLS is terminated. Serve the SPA
with a restrictive `Content-Security-Policy`.

---

### FINDING-7 — Database connections run in plaintext  ·  **MEDIUM**

**Category:** CWE-319 Cleartext Transmission of Sensitive Information

**Evidence.** `backend/.env` and `.env.example` set `DB_SSLMODE=disable`; the
live backend connection reports `PLAINTEXT` via `pg_stat_ssl`.

**Impact.** On localhost, negligible. Once the database is on a separate host or
managed service, credentials and every row (PII: passenger names, passport
numbers, contact details) cross the network unencrypted, exposed to anyone on
the path.

**Recommendation.** Use `sslmode=verify-full` with a pinned CA for any non-local
database. Keep `disable` only for the loopback dev setup, and document that the
production value must differ.

---

### FINDING-8 — Real database password committed in test source  ·  **LOW**

**Category:** CWE-798 Use of Hard-coded Credentials

**Evidence.** `backend/internal/config/config_test.go` contains the actual
development database password (redacted here; it was the live value from
`backend/.env`) as a test literal for the
DSN-escaping test.

**Impact.** Low today (dev password, not yet a git repository), but it will be
committed on first `git init`/push and then lives in history forever, including
if this password is reused elsewhere. Secrets in test fixtures are a common
real-world leak vector.

**Recommendation.** Replace with a synthetic password that still exercises the
escaping logic (e.g. `p@ss:w/rd?#%` — the test already covers such cases). Never
place a real, in-use secret in tracked source. Add a secret-scanning pre-commit
hook (e.g. gitleaks) before the first commit.

---

### FINDING-9 — Credentials stored reversibly client-side and replayed  ·  **LOW**

**Category:** CWE-522 Insufficiently Protected Credentials · CWE-312 Cleartext Storage

**Evidence.** `auth.service.ts` stores `btoa(email:password)` in `localStorage`
and the interceptor attaches it to every API call. base64 is encoding, not
encryption — the plaintext password is trivially recoverable from the browser.

**Impact.** This is inherent to HTTP Basic auth, and the README already flags it.
The concrete risk: any XSS (or a malicious/compromised dependency in the SPA)
reads the reusable password straight out of `localStorage` — worse than a
short-lived token would be — and the credential travels on every single request.

**Recommendation.** As the README foresees, move to short-lived tokens (JWT or
opaque session) with refresh, delivered in an `HttpOnly`, `Secure`, `SameSite`
cookie so client script cannot read them. The auth logic is already isolated to
`internal/middleware` + `internal/auth` and the Angular `auth.service`, so the
blast radius of the change is small.

---

### FINDING-10 — No server-side output encoding of stored data  ·  **LOW**

**Category:** CWE-79 (stored-XSS sink, currently unreachable)

**Evidence.** A passenger created with `firstName =
<script>alert(document.cookie)</script>` is stored and returned verbatim by the
API.

**Impact.** **Not currently exploitable** — every consumer is the Angular SPA,
which output-encodes on interpolation, and no `innerHTML` sink exists. The risk
is latent: a future consumer that renders this data as HTML (an email
confirmation, a PDF ticket, an admin tool, a report export) would execute it. It
is recorded as defence-in-depth.

**Recommendation.** Treat the API as the trust boundary: validate/normalise
free-text fields on input (reject or strip control characters and markup where a
name field cannot legitimately contain them), and ensure any non-Angular
renderer of this data encodes for its context. Do not rely solely on the
current client escaping.

---

### FINDING-11 — Role model implemented but unenforced  ·  **INFO**

**Category:** CWE-1220 Insufficient Granularity of Access Control (latent)

**Evidence.** `RequireAdmin` middleware exists and is unit-tested, and users
carry an `admin`/`agent` role, but `router.go` gates **no** route with it — every
authenticated user reaches every endpoint.

**Impact.** None today: there is no admin-only functionality to protect, so
`agent` and `admin` are functionally identical. Flagged because the moment an
admin-only capability is added (tenant settings, user management, refunds), it
must be remembered to attach `RequireAdmin`, or it will be silently open to all
agents. This is a broken-access-control regression waiting to happen.

**Recommendation.** When admin functionality is introduced, gate it explicitly
and add a test asserting an `agent` receives 403 — mirroring the existing
`TestRequireAdmin`. Consider a default-deny posture where new routes are
explicitly assigned a required role.

---

## 5. Remediation priority

| Priority | Findings | Rationale |
| -------- | -------- | --------- |
| **Do before any non-local deployment** | 1, 2, 4, 6, 7 | These convert from low-risk to serious the instant the app leaves localhost |
| **Do soon** | 3, 5, 8 | Cheap fixes that raise the cost of the attack paths above |
| **Do as the design evolves** | 9, 10, 11 | Tie to the planned auth change and to new features |

A single focused change set closes most of the high/medium items: introduce a
least-privilege DB role (1), add `SecureHeaders` + rate-limiting middleware to
the existing `Chain` (4, 6), align bcrypt cost and the dummy hash and move
hashing into Go (2, 3, 5), and flip `sslmode` for non-local (7).

---

## 6. Methodology notes & limitations

- Testing was against a single-node localhost deployment with seeded data. A
  production deployment behind a reverse proxy / WAF / TLS terminator may
  already mitigate some network-layer findings (6, 7) — verify against the real
  topology.
- Timing measurements were taken over loopback (mean of 30 requests); absolute
  values will differ over a network, but the *relative* discrepancies that
  constitute FINDING-2 and FINDING-5 are structural (different work factors /
  short-circuit paths) and will persist.
- No load, fuzzing, or dependency-CVE (SCA) scanning was performed; recommend
  adding `govulncheck` (Go) and `npm audit`/OWASP Dependency-Check (frontend)
  to CI as a follow-up.
- Scope was the application. Host hardening, PostgreSQL configuration
  (`pg_hba.conf`, listen addresses), and OS patching were out of scope.
```