Skip to content

fix(codegen-ui-react): validate schema-supplied strings before emitting identifiers - #1204

Merged
osama-rizk merged 1 commit into
mainfrom
fix/cve-2025-4318-codegen-injection-sweep
Aug 4, 2026
Merged

fix(codegen-ui-react): validate schema-supplied strings before emitting identifiers#1204
osama-rizk merged 1 commit into
mainfrom
fix/cve-2025-4318-codegen-injection-sweep

Conversation

@soberm

@soberm soberm commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Hardens @aws-amplify/codegen-ui-react against the CVE-2025-4318 code-injection class.

TypeScript's factory.createIdentifier(), createPropertyAccessChain() and createPropertyAccessExpression() emit their string argument verbatim as source. Where a component, view, form or theme schema string reaches one of those positions without validation, it is written directly into the generated .tsx as executable code rather than as data — so a malicious or malformed schema can inject arbitrary code into a generated component.

Earlier fixes for this class addressed individual call sites. This change covers the schema-derived emit sites across the package.

What is fixed

All schema-derived values are validated against an allowlist before emission. Sites now guarded include:

Component properties and conditions

  • buildConditionalExpressioncondition.property and condition.field
  • buildOpeningElementProperties — property key emitted as a JSX attribute name
  • buildOpeningElementEvents — unmapped event name emitted as a JSX attribute name
  • bindingProperties / collectionProperties keys emitted as generated declarations

Collections and data bindings

  • CollectionRenderer.findItemsVariableName — a collectionProperties key flowing into the collection's items={…} variable
  • buildUseEffectStatementsstateReference.dataDependencies, which carry bindingProperties.property and condition.property through to the useEffect guard and its dependency array
  • buildSortFunctionsort.field
  • predicateToObjectLiteralExpression (component and view) — predicate field, operator and keys

Views, tables and forms

  • ReactExpanderRenderer — componentSlot binding property/field, binding property keys, componentSlot.componentName, collection title binding field
  • createFieldAccessExpression and table body cell rendering — column field and header
  • objectToExpression — view fieldFormatting keys
  • buildDefaultModelDisplayValue — a relationship field's default displayValue mapping property and field
  • buildActionArgument / assignFieldProperties — action parameter keys

Compile-time constants and internally derived names are left unchanged.

Shared validators

Validation is consolidated in lib/utils/identifiers.ts rather than duplicated per call site. escapePropertyValue() now delegates to escapeIdentifierPath(), and buildThemePropertyName() to buildIdentifierOrStringLiteral(), removing three copies of the same allowlist regex.

One additional validator was necessary. JSX attribute names follow the JSXIdentifier grammar, which permits - — legitimate schemas use data-testid and aria-label — and forbids the dot-paths that property bindings allow. Validating attribute names with the plain-identifier rule would reject valid input, so JSX_ATTRIBUTE_NAME_RE covers that position specifically.

All Object.entries access to bindingProperties / collectionProperties routes through safeIdentifierEntries(), including sites that discard the key, so the guard cannot be bypassed by a new caller.

Fail-closed behavior

Each position fails closed in the way appropriate to its syntax, and no position emits an empty identifier — an empty identifier cannot execute, but it is unparseable and surfaces later as an opaque formatter error.

  • Positions accepting a literal emit a quoted string literal.
  • Bare-identifier and property-access-member positions throw, naming the offending schema field and value.
  • JSX attribute names have no literal form (JsxAttribute.name is typed as Identifier), so the attribute is omitted.
  • JSX element names likewise have no literal form and are rejected.
  • Collection item variables fall back to items.

Behavior for valid schemas is unchanged.

Testing

  • Adds lib/__tests__/cve-2025-4318-injection.test.ts with 31 tests covering each guarded position, including tests that exercise the full render pipeline rather than the AST printer alone.
  • 634 tests pass across 32 suites.
  • All 457 existing snapshots are byte-identical, confirming no change in output for valid input.

Changes are limited to source; build artifacts are regenerated.

@soberm
soberm requested a review from a team as a code owner July 31, 2026 11:33
@codecov-commenter

codecov-commenter commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.46154% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 93.96%. Comparing base (3918799) to head (d000c9e).

Files with missing lines Patch % Lines
...es/codegen-ui-react/lib/react-expander-renderer.ts 90.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1204      +/-   ##
==========================================
+ Coverage   93.92%   93.96%   +0.03%     
==========================================
  Files         151      151              
  Lines        6142     6180      +38     
  Branches     1842     1847       +5     
==========================================
+ Hits         5769     5807      +38     
  Misses        355      355              
  Partials       18       18              
Files with missing lines Coverage Δ
...en-ui-react/lib/amplify-ui-renderers/collection.ts 98.61% <100.00%> (+0.03%) ⬆️
.../codegen-ui-react/lib/amplify-ui-renderers/form.ts 95.94% <100.00%> (+0.05%) ⬆️
...act/lib/forms/form-renderer-helper/model-values.ts 92.57% <100.00%> (+0.08%) ⬆️
...egen-ui-react/lib/react-component-render-helper.ts 92.24% <100.00%> (+0.02%) ⬆️
...s/codegen-ui-react/lib/react-component-renderer.ts 100.00% <100.00%> (ø)
...react/lib/react-studio-template-renderer-helper.ts 94.31% <100.00%> (+0.06%) ⬆️
...gen-ui-react/lib/react-studio-template-renderer.ts 93.66% <100.00%> (+0.01%) ⬆️
...odegen-ui-react/lib/react-table-renderer-helper.ts 82.92% <100.00%> (+0.42%) ⬆️
...kages/codegen-ui-react/lib/react-table-renderer.ts 96.96% <100.00%> (+0.04%) ⬆️
...-react/lib/react-theme-studio-template-renderer.ts 92.50% <100.00%> (ø)
... and 6 more

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3918799...d000c9e. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@soberm soberm changed the title fix(codegen-ui-react): guard all schema-derived identifier emit sites (CVE-2025-4318) fix(codegen-ui-react): guard all schema-derived identifier emit sites Jul 31, 2026
…ng identifiers

factory.createIdentifier(), createPropertyAccessChain() and
createPropertyAccessExpression() emit their string argument verbatim as source.
Where a component, view, form or theme schema string reached one of those
positions without validation, it was written into the generated .tsx as
executable code rather than as data, allowing code injection into generated
components.

Earlier fixes addressed individual call sites. This change covers the
schema-derived emit sites across the package. All schema-derived values are now
validated against an allowlist before emission; compile-time constants and
internally derived names are unchanged.

Sites now guarded:

Component properties and conditions
- buildConditionalExpression: condition.property and condition.field
- buildOpeningElementProperties: property key -> JSX attribute name
- buildOpeningElementEvents: unmapped event name -> JSX attribute name
- bindingProperties / collectionProperties keys -> generated declarations

Collections and data bindings
- CollectionRenderer.findItemsVariableName: a collectionProperties key flowing
  into the collection's items={...} variable
- buildUseEffectStatements: stateReference dataDependencies, which carry
  bindingProperties.property and condition.property through to the useEffect
  guard and its dependency array
- buildSortFunction: sort.field
- predicateToObjectLiteralExpression (component and view): predicate field,
  operator and keys

Views, tables and forms
- ReactExpanderRenderer: componentSlot binding property/field, binding property
  keys, componentSlot.componentName, collection title binding field
- createFieldAccessExpression and table body cell: column field and header
- objectToExpression: view fieldFormatting keys
- buildDefaultModelDisplayValue: a relationship field's default displayValue
  mapping property and field
- buildActionArgument / assignFieldProperties: action parameter keys

Validation is consolidated in lib/utils/identifiers.ts rather than duplicated per
call site. escapePropertyValue() now delegates to escapeIdentifierPath() and
buildThemePropertyName() to buildIdentifierOrStringLiteral(), removing three
copies of the same allowlist regex. All Object.entries access to
bindingProperties / collectionProperties routes through safeIdentifierEntries(),
including sites that discard the key, so a new caller cannot bypass the guard.

One additional validator was necessary. JSX attribute names follow the
JSXIdentifier grammar, which permits '-' -- legitimate schemas use data-testid
and aria-label -- and forbids the dot-paths that property bindings allow.
Validating attribute names with the plain-identifier rule would reject valid
input, so JSX_ATTRIBUTE_NAME_RE covers that position specifically.

Each position fails closed in the way appropriate to its syntax, and no position
emits an empty identifier: an empty identifier cannot execute, but it is
unparseable and surfaces later as an opaque formatter error. Positions accepting
a literal emit a quoted string literal. Bare-identifier and
property-access-member positions throw, naming the offending schema field and
value. JSX attribute names have no literal form (JsxAttribute.name is typed as
Identifier), so the attribute is omitted. JSX element names likewise have no
literal form and are rejected. Collection item variables fall back to items.

Behavior for valid schemas is unchanged: all 457 existing snapshots are byte
identical. Adds lib/__tests__/identifier-injection.test.ts with 31 tests covering
each guarded position, including tests that exercise the full render pipeline
rather than the AST printer alone. 634 tests pass across 32 suites.

Changes are limited to source; build artifacts are regenerated.
@soberm
soberm force-pushed the fix/cve-2025-4318-codegen-injection-sweep branch from e605727 to d000c9e Compare July 31, 2026 11:47
@soberm soberm changed the title fix(codegen-ui-react): guard all schema-derived identifier emit sites fix(codegen-ui-react): validate schema-supplied strings before emitting identifiers Jul 31, 2026
@osama-rizk
osama-rizk merged commit 0f774ee into main Aug 4, 2026
14 of 15 checks passed
@osama-rizk
osama-rizk deleted the fix/cve-2025-4318-codegen-injection-sweep branch August 4, 2026 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants