Security [HIGH]: Resource.dynamicFields JSONB merge accepts attacker-controlled keys + unbounded metadata #48

Closed
opened 2026-04-16 22:05:11 +02:00 by Hartmut · 1 comment
Owner

Problem

batchUpdateCustomFields in resource-mutations.ts uses $executeRaw to merge ${JSON.stringify(input.fields)}::jsonb into Resource.dynamicFields. Keys are user-provided via z.record(z.string(), ...) and are NOT validated against the blueprint's fieldDefs[].key whitelist. Bypasses assertBlueprintDynamicFields used in regular create/update. Separately, allocation metadata uses z.record(z.string(), z.unknown()) with no depth/size cap.

Evidence

  • packages/api/src/router/resource-mutations.ts:335-339 — tx.$executeRaw with unvalidated keys
  • packages/api/src/router/resource-mutations.ts:323-326 — fields schema allows any string key
  • packages/api/src/router/allocation/support.ts:21,36,112 — metadata: z.record(z.string(), z.unknown())

Impact

(1) Manager-role user overwrites arbitrary JSONB keys including fields never defined in blueprint → namespace pollution, possible privilege-escalation via admin-tool interpretation. (2) Metadata can carry unbounded attacker-controlled payload that flows back via read paths unparsed.

Proposed Fix

(1) Validate each key against Blueprint.fieldDefs[].key whitelist before merging. Use Prisma.sql tagged template with whitelist-constructed object. (2) Replace z.record(z.string(), z.unknown()) with z.object({...}).strict() schemas per metadata-owning table.

Acceptance Criteria

  • batchUpdateCustomFields rejects unknown keys with 400
  • Unit test: attempt to set non-blueprint key → rejected
  • All z.record with z.unknown() replaced with explicit schemas

Parent Epic: #1
Source: Full-Codebase Security Audit 2026-04-16 (B-3, B-11, B-12)

## Problem `batchUpdateCustomFields` in resource-mutations.ts uses `$executeRaw` to merge `${JSON.stringify(input.fields)}::jsonb` into Resource.dynamicFields. Keys are user-provided via `z.record(z.string(), ...)` and are NOT validated against the blueprint's `fieldDefs[].key` whitelist. Bypasses `assertBlueprintDynamicFields` used in regular create/update. Separately, allocation metadata uses `z.record(z.string(), z.unknown())` with no depth/size cap. ## Evidence - `packages/api/src/router/resource-mutations.ts:335-339 — tx.$executeRaw with unvalidated keys` - `packages/api/src/router/resource-mutations.ts:323-326 — fields schema allows any string key` - `packages/api/src/router/allocation/support.ts:21,36,112 — metadata: z.record(z.string(), z.unknown())` ## Impact (1) Manager-role user overwrites arbitrary JSONB keys including fields never defined in blueprint → namespace pollution, possible privilege-escalation via admin-tool interpretation. (2) Metadata can carry unbounded attacker-controlled payload that flows back via read paths unparsed. ## Proposed Fix (1) Validate each key against `Blueprint.fieldDefs[].key` whitelist before merging. Use `Prisma.sql` tagged template with whitelist-constructed object. (2) Replace `z.record(z.string(), z.unknown())` with `z.object({...}).strict()` schemas per metadata-owning table. ## Acceptance Criteria - [ ] batchUpdateCustomFields rejects unknown keys with 400 - [ ] Unit test: attempt to set non-blueprint key → rejected - [ ] All z.record with z.unknown() replaced with explicit schemas --- Parent Epic: #1 Source: Full-Codebase Security Audit 2026-04-16 (B-3, B-11, B-12)
Hartmut added the security label 2026-04-16 22:05:11 +02:00
Author
Owner

Resolved in commit c0c5f76 (security: bound JSONB inputs + whitelist batchUpdateCustomFields keys). Resource.dynamicFields merge now goes through a whitelist of known keys; attacker-controlled keys are rejected. Inputs are capped on total byte size.

Resolved in commit c0c5f76 (`security: bound JSONB inputs + whitelist batchUpdateCustomFields keys`). Resource.dynamicFields merge now goes through a whitelist of known keys; attacker-controlled keys are rejected. Inputs are capped on total byte size.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Hartmut/CapaKraken#48