Initial commit: TravelHub multi-tenant flight booking platform
Angular + Go + PostgreSQL sample with strict per-tenant isolation. - Backend: Go API (basic auth, tenant-scoped repository layer, pgx pool) - Frontend: Angular 22 SPA (search, checkout, bookings, dashboard) - Database: schema, seed data and demo bookings migrations - Tests: 311 across Go (integration), Angular (unit) and Playwright (e2e), plus a generated evidence report with PDF export - Security: whitebox VAPT report under security/ Secrets (.env, tokens, DB credentials) are git-ignored. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
nilesh.p
Claude Opus 4.8
@@ -0,0 +1,396 @@
|
||||
# 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.
|
||||
```
|
||||
Reference in New Issue
Block a user