vuln fix
This commit is contained in:
root
@@ -0,0 +1,179 @@
|
||||
# Security Vulnerability Fix Report
|
||||
|
||||
**Date:** 2026-05-22
|
||||
**Scope:** API (`apps/api`) — automated scan + manual review
|
||||
**Branch:** `develop`
|
||||
|
||||
---
|
||||
|
||||
## Summary
|
||||
|
||||
A full security review of the API codebase identified 3 confirmed, high-confidence vulnerabilities. All 3 were fixed in the same session. A separate frontend scan (dashboard, admin, marketplace, public-site) found no confirmed vulnerabilities above the reporting threshold.
|
||||
|
||||
| # | Severity | Category | Status |
|
||||
|---|----------|----------|--------|
|
||||
| [VF-01](#vf-01--payment-credential-exposure) | High | Credential / Data Exposure | Fixed |
|
||||
| [VF-02](#vf-02--webhook-signature-bypass) | High | Authentication Bypass | Fixed |
|
||||
| [VF-03](#vf-03--idor-on-vehicle-maintenance-logs) | Medium-High | IDOR / Tenant Isolation | Fixed |
|
||||
|
||||
---
|
||||
|
||||
## VF-01 — Payment Credential Exposure
|
||||
|
||||
**Severity:** High
|
||||
**Confidence:** 9/10
|
||||
**Category:** Credential / Data Exposure
|
||||
|
||||
### Description
|
||||
|
||||
`GET /api/v1/companies/me/brand` returned the full `BrandSettings` database row, including the live payment gateway credentials `amanpaySecretKey`, `amanpayMerchantId`, `paypalEmail`, and `paypalMerchantId` in the JSON response. The endpoint had no role guard, so any authenticated employee — including the lowest-privilege `AGENT` role — could call it and receive the secret key in plaintext.
|
||||
|
||||
### Exploit Scenario
|
||||
|
||||
An `AGENT`-role employee calls `GET /api/v1/companies/me/brand` with a valid session token. The response body includes `amanpaySecretKey`. The attacker uses the merchant ID and secret key to interact directly with the AmanPay payment gateway on behalf of the company, initiating fraudulent charges or refunds outside the application.
|
||||
|
||||
### Root Cause
|
||||
|
||||
`presentBrand()` in `company.presenter.ts` was a transparent passthrough (`return brand`). The DB query had no `select` clause, so Prisma returned every column. The public-facing site module correctly stripped these fields (evidenced by explicit test assertions), but the authenticated dashboard endpoint did not.
|
||||
|
||||
### Fix
|
||||
|
||||
**File:** `apps/api/src/modules/companies/company.presenter.ts`
|
||||
|
||||
`presentBrand()` now destructures and drops all four credential fields before returning. The caller receives boolean presence indicators (`amanpayConfigured`, `paypalConfigured`) instead of the raw secrets.
|
||||
|
||||
```diff
|
||||
export function presentBrand(brand: any) {
|
||||
- return brand
|
||||
+ if (!brand) return brand
|
||||
+ const { amanpaySecretKey, amanpayMerchantId, paypalEmail, paypalMerchantId, ...safe } = brand
|
||||
+ return {
|
||||
+ ...safe,
|
||||
+ amanpayConfigured: !!(amanpayMerchantId && amanpaySecretKey),
|
||||
+ paypalConfigured: !!(paypalEmail || paypalMerchantId),
|
||||
+ }
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## VF-02 — Webhook Signature Bypass
|
||||
|
||||
**Severity:** High
|
||||
**Confidence:** 8/10
|
||||
**Category:** Authentication Bypass / Payment Fraud
|
||||
|
||||
### Description
|
||||
|
||||
Both payment and subscription webhook endpoints for AmanPay and PayPal used a short-circuit AND guard for signature verification:
|
||||
|
||||
```ts
|
||||
if (amanpay.isConfigured() && !amanpay.verifyWebhookSignature(rawBody, signature)) {
|
||||
return res.status(401).json({ error: 'invalid_signature' })
|
||||
}
|
||||
await service.handleAmanpayWebhook(req.body) // executed unconditionally
|
||||
```
|
||||
|
||||
When `isConfigured()` returns `false` (i.e., credentials are absent or set to placeholder defaults), the entire guard short-circuits and the webhook handler executes on any unauthenticated request. The endpoints had no IP allowlist and were explicitly placed before `requireCompanyAuth`.
|
||||
|
||||
This affected 4 endpoints:
|
||||
- `POST /api/v1/payments/webhooks/amanpay`
|
||||
- `POST /api/v1/payments/webhooks/paypal`
|
||||
- `POST /api/v1/subscriptions/webhooks/amanpay`
|
||||
- `POST /api/v1/subscriptions/webhooks/paypal`
|
||||
|
||||
### Exploit Scenario
|
||||
|
||||
On any staging or misconfigured production environment where AmanPay credentials are not set, an attacker posts `{"transaction_id": "<known_id>", "status": "PAID"}` to `/api/v1/payments/webhooks/amanpay`. The handler finds the pending payment by `transactionId`, marks it succeeded, and unlocks the reservation — with no money changing hands. A valid `amanpayTransactionId` can leak through receipts, logs, or API responses visible to renters.
|
||||
|
||||
The same attack against `/api/v1/subscriptions/webhooks/amanpay` activates a paid subscription tier for free.
|
||||
|
||||
### Fix
|
||||
|
||||
**Files:** `apps/api/src/modules/payments/payment.routes.ts`, `apps/api/src/modules/subscriptions/subscription.routes.ts`
|
||||
|
||||
Inverted the guard logic: an unconfigured provider now immediately returns `401` instead of passing through. Applied to all 4 endpoints.
|
||||
|
||||
```diff
|
||||
-if (amanpay.isConfigured() && !amanpay.verifyWebhookSignature(rawBody, signature)) {
|
||||
+if (!amanpay.isConfigured() || !amanpay.verifyWebhookSignature(rawBody, signature)) {
|
||||
return res.status(401).json({ error: 'invalid_signature' })
|
||||
}
|
||||
```
|
||||
|
||||
```diff
|
||||
-if (paypal.isConfigured() && !isValid) return res.status(401).json({ error: 'invalid_signature' })
|
||||
+if (!paypal.isConfigured() || !isValid) return res.status(401).json({ error: 'invalid_signature' })
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## VF-03 — IDOR on Vehicle Maintenance Logs
|
||||
|
||||
**Severity:** Medium-High
|
||||
**Confidence:** 9/10
|
||||
**Category:** Insecure Direct Object Reference (IDOR) / Tenant Isolation Bypass
|
||||
|
||||
### Description
|
||||
|
||||
`GET /api/v1/vehicles/:id/maintenance` was protected by `requireCompanyAuth` and `requireTenant`, making `req.companyId` available. However, `req.companyId` was never forwarded to the service or repository. The database query used only `vehicleId` with no tenant filter:
|
||||
|
||||
```ts
|
||||
// vehicle.repo.ts
|
||||
prisma.maintenanceLog.findMany({ where: { vehicleId } })
|
||||
```
|
||||
|
||||
An authenticated employee from Company A could fetch the full maintenance history of any vehicle belonging to any other company by knowing its UUID. The write path (`POST /:id/maintenance`) already performed the correct ownership check using `repo.findById(id, companyId)` — the read path did not.
|
||||
|
||||
### Exploit Scenario
|
||||
|
||||
An employee of Company A calls `GET /api/v1/vehicles/<vehicleId_from_company_B>/maintenance`. Vehicle UUIDs can leak through marketplace listings, shared bookings, or support interactions. The response contains the complete maintenance history of Company B's vehicle — including service records, mileage data, and operational schedules — across tenant boundaries.
|
||||
|
||||
### Fix
|
||||
|
||||
**Files:** `apps/api/src/modules/vehicles/vehicle.routes.ts`, `apps/api/src/modules/vehicles/vehicle.service.ts`
|
||||
|
||||
`req.companyId` is now threaded through to `getMaintenanceLogs`, which performs the same ownership pre-check already used by the write path. A cross-tenant request returns `404`.
|
||||
|
||||
```diff
|
||||
// vehicle.routes.ts
|
||||
-const logs = await service.getMaintenanceLogs(id)
|
||||
+const logs = await service.getMaintenanceLogs(id, req.companyId)
|
||||
```
|
||||
|
||||
```diff
|
||||
// vehicle.service.ts
|
||||
-export async function getMaintenanceLogs(id: string) {
|
||||
- return repo.findMaintenanceLogs(id)
|
||||
+export async function getMaintenanceLogs(id: string, companyId: string) {
|
||||
+ const vehicle = await repo.findById(id, companyId)
|
||||
+ if (!vehicle) throw new NotFoundError('Vehicle not found')
|
||||
+ return repo.findMaintenanceLogs(id)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Frontend Scan Results
|
||||
|
||||
A full scan of all four frontend applications (dashboard, admin, marketplace, public-site) was performed. No vulnerabilities above the reporting threshold (confidence ≥ 8/10) were confirmed.
|
||||
|
||||
### Hardening Recommendations (not vulnerabilities)
|
||||
|
||||
Two items were identified as best-practice improvements:
|
||||
|
||||
1. **Admin auth-redirect `next` param** (`apps/admin/src/app/auth-redirect/page.tsx`) — Validate the `next` query parameter is a relative path (starts with `/`, does not contain `://`) before passing it to `router.replace`. The router itself does not perform cross-origin navigation, but defense-in-depth is recommended.
|
||||
|
||||
2. **`postMessage` wildcard origin + missing `frame-ancestors` CSP** (`apps/dashboard/src/app/sign-in/[[...sign-in]]/SignInPageClient.tsx`, `apps/dashboard/next.config.js`) — Replace `targetOrigin: '*'` with the marketplace origin, and add `Content-Security-Policy: frame-ancestors 'self' <marketplace-origin>` to prevent the sign-in page from being embeddable by arbitrary third-party domains.
|
||||
|
||||
---
|
||||
|
||||
## Files Changed
|
||||
|
||||
| File | Change |
|
||||
|------|--------|
|
||||
| `apps/api/src/modules/companies/company.presenter.ts` | Strip payment credentials in `presentBrand()` |
|
||||
| `apps/api/src/modules/payments/payment.routes.ts` | Fix webhook signature bypass (AmanPay + PayPal) |
|
||||
| `apps/api/src/modules/subscriptions/subscription.routes.ts` | Fix webhook signature bypass (AmanPay + PayPal) |
|
||||
| `apps/api/src/modules/vehicles/vehicle.routes.ts` | Pass `companyId` to maintenance log service |
|
||||
| `apps/api/src/modules/vehicles/vehicle.service.ts` | Add ownership check in `getMaintenanceLogs` |
|
||||
Reference in New Issue
Block a user