From 5b4a714680efa53e21b049e6a59bf10bf361b9f0 Mon Sep 17 00:00:00 2001 From: brunozoric Date: Mon, 27 Jul 2026 10:16:55 +0200 Subject: [PATCH 1/2] chore: event handler security issues --- .../security/event-handler-attack-surface.md | 210 ++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 docs/.bruno/security/event-handler-attack-surface.md diff --git a/docs/.bruno/security/event-handler-attack-surface.md b/docs/.bruno/security/event-handler-attack-surface.md new file mode 100644 index 00000000000..eb68818ec95 --- /dev/null +++ b/docs/.bruno/security/event-handler-attack-surface.md @@ -0,0 +1,210 @@ +# Security Audit: Event Handler Attack Surface + +**Date:** 2026-07-27 +**Packages:** `event-handler-aws`, `event-handler-core`, `api-event-handler-aws` +**Findings:** 6 (1 high, 2 medium, 3 low) +**Tests:** 6 (proving finding #1) + +--- + +## Audit Scope + +- `packages/event-handler-aws/src/` — Lambda transport, event types, translators, handlers +- `packages/event-handler-core/src/` — HTTP abstractions, event handler interfaces +- `packages/api-event-handler-aws/src/` — Identity & tenant loader decorators + +--- + +## Finding #1 — Prototype Pollution via Unsanitized JSON.parse + +**Severity:** High +**CWE:** CWE-1321 — Improperly Controlled Modification of Object Prototype Attributes +**Status:** Proven with tests +**File:** `event-handler-aws/src/translators/apiGatewayEventToHttpRequest.ts:17` + +### Problem + +`JSON.parse(event.body)` runs with no sanitization. An attacker sends `{"__proto__":{"isAdmin":true}}` as the request body. `JSON.parse` creates `__proto__` as an **own property** on the result object. + +When any downstream code recursively merges this object — `lodash.merge`, `deepmerge`, GraphQL context assembly, custom config builders — it walks into `target["__proto__"]`, which resolves to `Object.prototype` via the getter. The attacker's keys are now on **every object in the runtime**. + +### Attack Flow + +``` +Attacker HTTP body → JSON.parse → request.body → recursive merge → Object.prototype polluted +(raw JSON string) (__proto__ (passed (lodash.merge (all objects + own prop) downstream) etc) affected) +``` + +### Proven Attack Vectors + +All proven with tests in `event-handler-aws/__tests__/apiGatewayEventToHttpRequest.security.test.ts`: + +1. **`__proto__` pollution** (v1 + v2 event formats) — global `Object.prototype` poisoned via recursive merge +2. **`constructor.prototype` pollution** — alternate path, same outcome +3. **Deep nesting** (10,000 levels) — downstream recursive processors stack-overflow +4. **Excessive key count** (100k keys) — hash-flood DoS +5. **Silent error swallowing** — malformed JSON becomes raw string with no error signal, causing type confusion downstream + +### Recommended Fix (Option A — preferred): `secure-json-parse` + +Battle-tested drop-in for `JSON.parse`. Zero dependencies, ~2 KB. Used by Fastify (billions of requests/day). + +```bash +yarn workspace @webiny/event-handler-aws add secure-json-parse +``` + +```ts +import sjson from "secure-json-parse"; + +// In apiGatewayEventToHttpRequest.ts: +if (event.body) { + try { + body = sjson.parse(event.body, undefined, { + protoAction: "remove", // strips __proto__ keys + constructorAction: "remove" // strips constructor keys + }); + } catch { + body = event.body; + } +} +``` + +`protoAction` / `constructorAction` accept `"remove"` (silent strip), `"error"` (throw on detection), or `"ignore"` (default — current behavior). + +### Recommended Fix (Option B — zero dependencies): JSON.parse reviver + +```ts +function safeParse(raw: string): unknown { + return JSON.parse(raw, (key, value) => { + if (key === "__proto__" || key === "constructor") { + return undefined; + } + return value; + }); +} +``` + +The reviver runs bottom-up for every key, so nested paths like `{"a":{"__proto__":{"x":1}}}` are caught. + +**Caveat:** The reviver strips ALL `constructor` keys, including legitimate ones. `secure-json-parse` only strips `constructor` when it contains a `prototype` sub-key — more precise. + +--- + +## Finding #2 — Internal Error Details Leaked to HTTP Response + +**Severity:** Medium +**CWE:** CWE-209 — Generation of Error Message Containing Sensitive Information +**File:** `event-handler-aws/src/handlers/ApiGatewayHttpRouterHandler.ts:23-31` + +### Problem + +When a route handler throws an error with a `code` property, the catch block returns `message`, `code`, and `data` verbatim in the 500 response body. Stack traces, database error codes, and internal data structures reach the attacker, aiding further exploitation. + +```ts +// Current: leaks internals +return httpResponseToApiGatewayResult({ + statusCode: 500, + body: { + message: (e as any).message, // ← may contain SQL, stack trace + code: (e as any).code, // ← internal error taxonomy + data: (e as any).data ?? null // ← arbitrary internal data + } +}); +``` + +### Recommended Fix + +Log full error server-side. Return only a generic message to the client. + +```ts +catch (e) { + console.error("HTTP handler error:", e); + return httpResponseToApiGatewayResult({ + statusCode: 500, + body: { message: "Internal server error" } + }); +} +``` + +--- + +## Finding #3 — Tenant Header Trusted Without Validation + +**Severity:** Medium +**CWE:** CWE-20 — Improper Input Validation +**File:** `api-event-handler-aws/src/handlers/ApiGatewayTenantLoaderDecorator.ts:27` + +### Problem + +The `x-tenant` header value is read and passed to `RawTenantId.set()` with no format validation, length limit, or character restriction. If downstream code uses this value in database queries, path construction, or cache keys without its own sanitization, it becomes an injection vector. Header spoofing is trivial — no authentication is required to set arbitrary headers. + +```ts +// Current: no validation +this.rawTenantId.set( + headers ? (headers["x-tenant"] ?? headers["X-Tenant"] ?? null) : null +); +``` + +### Recommended Fix + +Validate tenant ID against expected format before passing downstream. + +```ts +const TENANT_RE = /^[a-zA-Z0-9_-]{1,64}$/; + +const raw = headers?.["x-tenant"] ?? headers?.["X-Tenant"] ?? null; +const tenantId = raw && TENANT_RE.test(raw) ? raw : null; +this.rawTenantId.set(tenantId); +``` + +--- + +## Finding #4 — No Auth Token Length or Character Validation + +**Severity:** Low +**CWE:** CWE-20 — Improper Input Validation +**File:** `api-event-handler-aws/src/handlers/ApiGatewayIdentityLoaderDecorator.ts:47-55` + +Bearer token extracted via regex and passed to the identity loader with no upper bound on length or character set restriction. An oversized token (multi-MB) could cause memory pressure in the auth pipeline. Impact depends on downstream identity loader implementation. + +--- + +## Finding #5 — Open Index Signature on WebSocket Event Interface + +**Severity:** Low +**CWE:** CWE-1321 +**File:** `event-handler-aws/src/eventTypes/WebSocketEventType.ts:13` + +`IWebSocketEvent.requestContext` declares `[key: string]: unknown`, accepting arbitrary keys. Combined with body parsing (body is `string | Record`), same prototype pollution class applies if body is ever JSON.parsed downstream. TypeScript won't flag pollution-prone keys at compile time. + +--- + +## Finding #6 — Raw Lambda Event Registered into DI Container + +**Severity:** Low +**CWE:** CWE-20 — Improper Input Validation +**File:** `event-handler-aws/src/AwsLambdaTransport.ts:19` + +The entire unsanitized Lambda event is registered as `AwsLambdaEvent` in the DI container. Every consumer that resolves this abstraction receives attacker-controlled body, headers, query params, and path parameters with no trust boundary. This is the systemic root of findings #1, #3, and #4 — sanitization should happen at registration, not per-consumer. + +--- + +## Priority + +| Priority | Finding | Action | +|----------|---------|--------| +| **High** | #1 Prototype pollution | Exploitable now. Proven with tests. Fix first. | +| **Medium** | #2 Error leakage | Information disclosure aids further attacks. Quick fix. | +| **Medium** | #3 Tenant header | Depends on downstream validation. Validate at boundary. | +| **Low** | #4 #5 #6 | Require specific downstream conditions. Address in hardening pass. | + +--- + +## Test Coverage + +Six security tests in `event-handler-aws/__tests__/apiGatewayEventToHttpRequest.security.test.ts` proving all five attack vectors for Finding #1. + +```bash +yarn test packages/event-handler-aws +``` From 43b5f108eb6301ab46b91f78bf9d0456d916fdc1 Mon Sep 17 00:00:00 2001 From: brunozoric Date: Mon, 27 Jul 2026 11:02:06 +0200 Subject: [PATCH 2/2] chore: add Object.assign security audit map Classifies all 22 Object.assign call sites in API packages by prototype pollution risk. No direct RISK found; 3 INDIRECT cases documented on watch list. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/.bruno/security/object-assign-map.md | 95 +++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 docs/.bruno/security/object-assign-map.md diff --git a/docs/.bruno/security/object-assign-map.md b/docs/.bruno/security/object-assign-map.md new file mode 100644 index 00000000000..f9fcedbd93a --- /dev/null +++ b/docs/.bruno/security/object-assign-map.md @@ -0,0 +1,95 @@ +# Object.assign Audit — API Packages + +**Date:** 2026-07-27 +**Related to:** [Event Handler Attack Surface](./event-handler-attack-surface.md) (Finding #1 — Prototype Pollution) +**Scope:** All `Object.assign` calls in `packages/api-*` source files (excluding tests) +**Total:** 22 call sites across 18 files + +--- + +## Summary + +| Classification | Count | Description | +|---|---|---| +| **SAFE** | 14 | Source is internal-only (model defs, config, constants, internal events) | +| **INDIRECT** | 8 | User data arrives but through validated/constrained path | +| **RISK** | 0 | No direct user-to-Object.assign path found | + +No immediate action required, but 3 INDIRECT cases warrant monitoring. + +--- + +## Watch List (INDIRECT — closest to risk) + +### transformWhereToNested.ts:55 — GraphQL where clause expansion + +``` +packages/api-headless-cms/src/graphql/schema/cms/helpers/transformWhereToNested.ts:55 +``` + +Splits dot-notation keys from GraphQL `args.where` into nested objects. Pattern `result[head] = {}` on line 48-49 would invoke the `__proto__` setter if head were `"__proto__"`, changing the local object's prototype (DoS, not global pollution). **Mitigated** by GraphQL input type validation rejecting unknown field names. + +> **Fixed in `release/6.5.0`** ([webiny-js@release/6.5.0](https://github.com/webiny/webiny-js/blob/release/6.5.0/packages/api-headless-cms/src/graphql/schema/cms/helpers/transformWhereToNested.ts)): adds `FORBIDDEN_KEYS` set (`__proto__`, `prototype`, `constructor`), throws on forbidden keys, uses `Object.hasOwn` instead of bracket assignment. Needs backport to `next`. + +### searchableJsonFilterCreate.ts:9 / searchableJsonFilterCreateHandler.ts:8 — dotFlatten + +``` +packages/api-headless-cms-storage/src/filtering/plugins/searchableJsonFilterCreate.ts:9 +packages/api-headless-cms-storage/src/handlers/searchableJsonFilterCreateHandler.ts:8 +``` + +`Object.assign(acc, dotFlatten(val, path))` processes user-provided where-clause values. **Mitigated** by key prefixing during recursion — bare `__proto__` never appears as a top-level key in the result. + +### BaseModel.populate — public method, skippable validation + +``` +packages/api-core/src/models/base/BaseModel.ts:25 +``` + +`Object.assign(this, data)` in `populate()`. Normal path goes through Zod `safeParse` first (strips unknown keys). But `populate()` is public — a direct call bypasses validation. **Mitigated** by convention, not enforcement. + +--- + +## Full Classification + +### SAFE (14 call sites) + +| File | Line | Reason | +|---|---|---| +| `api-aco/src/utils/pickEntryFieldValues.ts` | 20 | Keys from hardcoded `baseFields` array | +| `api-audit-logs/src/context/AuditLogsContextValue.ts` | 62 | Internal event subscribers only | +| `api-audit-logs/src/context/AuditLogsContextValue.ts` | 98 | Internal event subscribers only | +| `api-core/src/features/users/shared/loaders.ts` | 85 | Target is fresh `{}`, source from DataLoader cache | +| `api-core/src/models/base/ModelBuilder.ts` | 80 | Developer-defined methods at setup time | +| `api-core/src/models/base/ModelBuilder.ts` | 112 | Developer-defined methods onto prototype | +| `api-core/src/models/cms/PrivateCmsModelBuilder.ts` | 72 | Developer-defined methods at setup time | +| `api-headless-cms-storage/src/filtering/fields/createFields.ts` | 91 | Model schema metadata, not entry data | +| `api-headless-cms-utils-os/src/operations/entry/elasticsearch/fields.ts` | 210 | Field metadata from model config | +| `api-headless-cms/src/constants.ts` | 91 | Keys from hardcoded `ENTRY_META_FIELDS` | +| `api-headless-cms/src/fieldConverters/CmsModelDynamicZoneFieldConverterPlugin.ts` | 180 | Source is storage layer (database) | +| `api-headless-cms/src/graphql/schema/createFieldResolvers.ts` | 82 | Plugin-registered resolver config | +| `api-workflows/src/domain/workflowState/WorkflowState.ts` | 328 | Hardcoded state objects within class | +| `api-workflows/src/domain/workflowState/WorkflowState.ts` | 336 | Hardcoded state transitions within class | + +### INDIRECT (8 call sites) + +| File | Line | Source | Mitigation | +|---|---|---|---| +| `api-core/src/features/tenancy/UpdateTenant/UpdateTenantUseCase.ts` | 31 | GraphQL mutation input | GraphQL input type constrains shape to known Tenant fields | +| `api-core/src/models/base/BaseModel.ts` | 25 | `populate(data)` — public method | Zod `safeParse` strips unknown keys on normal path; direct calls bypass | +| `api-file-manager-s3/src/utils/FileNormalizer.ts` | 58 | GraphQL file upload mutation | Source of assign is `modifier()` return (plugin), not raw user data | +| `api-file-manager/src/features/upload/utils/FileNormalizer.ts` | 57 | GraphQL file upload mutation | Same — plugin modifier controls returned keys | +| `api-headless-cms-storage/.../searchableJsonFilterCreate.ts` | 9 | GraphQL where clause values | `dotFlatten` prefixes all keys, prevents bare `__proto__` | +| `api-headless-cms-storage/.../searchableJsonFilterCreateHandler.ts` | 8 | GraphQL where clause values | Same key-prefixing mitigation | +| `api-headless-cms/.../CmsModelDynamicZoneFieldConverterPlugin.ts` | 98 | CMS entry values (GraphQL mutation) | Converter produces keys from model schema storage IDs | +| `api-headless-cms/.../transformWhereToNested.ts` | 55 | GraphQL `args.where` | GraphQL input type rejects unknown field names | + +--- + +## Conclusion + +All 22 `Object.assign` calls in API packages are either safe or mitigated by upstream validation (primarily GraphQL input type constraints). No direct exploit path exists today. + +**However**, these mitigations are defense-in-depth — they rely on GraphQL schema validation preventing `__proto__` from reaching the code. Fixing the root cause (Finding #1 in `apiGatewayEventToHttpRequest.ts`) removes the prototype pollution at the source, making these downstream sites safe regardless of their own validation. + +**Recommendation:** Fix Finding #1 first. The Object.assign sites don't need individual changes — they're safe once the input boundary is hardened.