Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit ca98c38

Browse files
authoredApr 10, 2025··
fix(codegen-ui-react): add sanitization to binding expression (#1174)
* fix(codegen-ui-react): add sanitization to binding expression * fix: address comments * fix: address comments
1 parent bbc3435 commit ca98c38

File tree

4 files changed

+426
-10
lines changed

4 files changed

+426
-10
lines changed
 

‎packages/codegen-ui-react/lib/__tests__/__snapshots__/react-component-render-helper.test.ts.snap

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`react-component-render-helper buildBindingExpression should generate snapshot for property with field 1`] = `"user?.address"`;
4+
5+
exports[`react-component-render-helper buildBindingExpression should generate snapshot for property with nested field access 1`] = `"data?.nested.field"`;
6+
7+
exports[`react-component-render-helper buildBindingExpression should generate snapshot for simple property 1`] = `"simpleProperty"`;
8+
39
exports[`react-component-render-helper buildChildElement bound property 1`] = `
410
NodeObject {
511
"dotDotDotToken": undefined,

‎packages/codegen-ui-react/lib/__tests__/react-component-render-helper.test.ts

Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ComponentMetadata,
2121
ConcatenatedStudioComponentProperty,
2222
} from '@aws-amplify/codegen-ui';
23+
import { factory } from 'typescript';
2324
import {
2425
getFixedComponentPropValueExpression,
2526
getComponentPropName,
@@ -36,6 +37,10 @@ import {
3637
hasChildrenProp,
3738
buildConcatExpression,
3839
parseNumberOperand,
40+
getStateName,
41+
escapePropertyValue,
42+
buildBindingExpression,
43+
filterScriptingPatterns,
3944
} from '../react-component-render-helper';
4045

4146
import { assertASTMatchesSnapshot } from './__utils__';
@@ -357,4 +362,272 @@ describe('react-component-render-helper', () => {
357362
);
358363
});
359364
});
365+
366+
describe('getStateName', () => {
367+
it('should correctly format state name by combining component name and property', () => {
368+
const stateReference = {
369+
componentName: 'UserProfile',
370+
property: 'firstName',
371+
};
372+
373+
const result = getStateName(stateReference);
374+
375+
expect(result).toBe('userProfileFirstName');
376+
});
377+
378+
it('should handle single word component names and properties', () => {
379+
const stateReference = {
380+
componentName: 'Button',
381+
property: 'active',
382+
};
383+
384+
const result = getStateName(stateReference);
385+
386+
expect(result).toBe('buttonActive');
387+
});
388+
389+
it('should handle empty strings', () => {
390+
const stateReference = {
391+
componentName: '',
392+
property: '',
393+
};
394+
395+
const result = getStateName(stateReference);
396+
397+
expect(result).toBe('');
398+
});
399+
400+
it('should handle special characters if sanitizeName is implemented', () => {
401+
const stateReference = {
402+
componentName: 'User$Profile',
403+
property: 'first@Name',
404+
};
405+
406+
const result = getStateName(stateReference);
407+
408+
expect(result).toBe('userDollarProfileFirstAtSymbolName');
409+
});
410+
});
411+
412+
describe('filterScriptingPatterns', () => {
413+
it('should keep alphanumeric characters', () => {
414+
expect(filterScriptingPatterns('abc123')).toBe('abc123');
415+
expect(filterScriptingPatterns('ABC789')).toBe('ABC789');
416+
});
417+
418+
it('should keep allowed special characters', () => {
419+
expect(filterScriptingPatterns('hello.world')).toBe('hello.world');
420+
expect(filterScriptingPatterns('first,second')).toBe('first,second');
421+
expect(filterScriptingPatterns('under_score')).toBe('under_score');
422+
expect(filterScriptingPatterns('dash-here')).toBe('dash-here');
423+
});
424+
425+
it('should remove disallowed special characters', () => {
426+
expect(filterScriptingPatterns('hello@world')).toBe('hello@world');
427+
expect(filterScriptingPatterns('test#123')).toBe('test#123');
428+
expect(filterScriptingPatterns('special!chars')).toBe('special!chars');
429+
expect(filterScriptingPatterns('remove$signs')).toBe('remove$signs');
430+
});
431+
432+
it('should handle spaces correctly', () => {
433+
expect(filterScriptingPatterns('hello world')).toBe('hello world');
434+
expect(filterScriptingPatterns(' extra spaces ')).toBe('extra spaces');
435+
expect(filterScriptingPatterns('\ttab\nspace')).toBe('tab\nspace');
436+
});
437+
438+
it('should handle eval strings', () => {
439+
expect(filterScriptingPatterns("eval('if(!window.x){alert(document.domain);window.x=1}')")).toBe('');
440+
// eslint-disable-next-line no-script-url
441+
expect(filterScriptingPatterns('javascript:alert(1)')).toBe('');
442+
});
443+
444+
it('should handle empty strings', () => {
445+
expect(filterScriptingPatterns('')).toBe('');
446+
expect(filterScriptingPatterns(' ')).toBe('');
447+
});
448+
449+
it('should handle non-string inputs', () => {
450+
expect(filterScriptingPatterns(null as any)).toBe('');
451+
expect(filterScriptingPatterns(undefined as any)).toBe('');
452+
});
453+
454+
it('should handle mixed content correctly', () => {
455+
expect(filterScriptingPatterns('Hello, World! @ #123')).toBe('Hello, World! @ #123');
456+
expect(filterScriptingPatterns('user.name@domain.com')).toBe('user.name@domain.com');
457+
expect(filterScriptingPatterns('path/to/file.txt')).toBe('path/to/file.txt');
458+
});
459+
460+
it('should preserve multiple allowed special characters', () => {
461+
expect(filterScriptingPatterns('item1,item2.item3-item4_item5')).toBe('item1,item2.item3-item4_item5');
462+
});
463+
464+
it('should handle unicode characters', () => {
465+
expect(filterScriptingPatterns('héllo wörld')).toBe('héllo wörld');
466+
expect(filterScriptingPatterns('←↑→↓')).toBe('←↑→↓');
467+
expect(filterScriptingPatterns('🌟star')).toBe('🌟star');
468+
});
469+
470+
it('should handle complex combinations', () => {
471+
const complexInput = `
472+
Hello! This is a "complex" test-case...
473+
With @multiple# lines & special chars.
474+
123_456-789.000
475+
`;
476+
477+
expect(filterScriptingPatterns(complexInput)).toBe(
478+
`Hello! This is a "complex" test-case...
479+
With @multiple# lines & special chars.
480+
123_456-789.000`,
481+
);
482+
});
483+
});
484+
485+
describe('escapePropertyName', () => {
486+
describe('Reserved Keywords', () => {
487+
it('should append "Prop" to JavaScript reserved keywords', () => {
488+
expect(escapePropertyValue('class')).toBe('classProp');
489+
expect(escapePropertyValue('function')).toBe('functionProp');
490+
expect(escapePropertyValue('var')).toBe('varProp');
491+
});
492+
493+
it('should not modify non-reserved words', () => {
494+
expect(escapePropertyValue('user')).toBe('user');
495+
expect(escapePropertyValue('name')).toBe('name');
496+
expect(escapePropertyValue('address')).toBe('address');
497+
});
498+
});
499+
500+
describe('Sanitization', () => {
501+
it('should sanitize invalid JavaScript identifiers', () => {
502+
expect(escapePropertyValue('user-name')).toBe('user-name');
503+
expect(escapePropertyValue('first.last')).toBe('first.last');
504+
expect(escapePropertyValue('special@char')).toBe('special@char');
505+
});
506+
507+
it('should handle spaces in property names', () => {
508+
expect(escapePropertyValue('first name')).toBe('first name');
509+
expect(escapePropertyValue(' spaced ')).toBe('spaced');
510+
});
511+
512+
it('should preserve valid characters', () => {
513+
expect(escapePropertyValue('validName123')).toBe('validName123');
514+
expect(escapePropertyValue('_privateVar')).toBe('_privateVar');
515+
expect(escapePropertyValue('$specialVar')).toBe('$specialVar');
516+
});
517+
});
518+
});
519+
520+
describe('buildBindingExpression', () => {
521+
it('should return an empty string with dangerous text', () => {
522+
const propertyValue = "eval('if(!window.x){alert(document.domain);window.x=1}')";
523+
const prop = {
524+
bindingProperties: {
525+
property: propertyValue,
526+
},
527+
};
528+
529+
const result = buildBindingExpression(prop);
530+
531+
expect((result as any).text).toBe('');
532+
});
533+
534+
it('should create property access chain for data attributes', () => {
535+
const prop = {
536+
bindingProperties: {
537+
property: 'value',
538+
field: 'data-test',
539+
},
540+
};
541+
542+
const result = buildBindingExpression(prop);
543+
544+
expect(result.kind).toBe(204);
545+
expect((result as any).name.escapedText).toBe('data-test');
546+
});
547+
548+
it('should return original string containing similar, but not dangerous text', () => {
549+
const propertyValue = 'evaluate if window alert document domain window';
550+
const prop = {
551+
bindingProperties: {
552+
property: propertyValue,
553+
},
554+
};
555+
556+
const result = buildBindingExpression(prop);
557+
558+
expect((result as any).text).toBe(propertyValue);
559+
});
560+
561+
it('should create a simple identifier when no field is present', () => {
562+
const prop = {
563+
bindingProperties: {
564+
property: 'userName',
565+
},
566+
};
567+
568+
const result = buildBindingExpression(prop);
569+
570+
// Check that it's an identifier with the correct text
571+
expect(result.kind).toBe(factory.createIdentifier('').kind);
572+
expect((result as any).text).toBe('userName');
573+
});
574+
575+
it('should handle reserved JavaScript keywords in property names', () => {
576+
const prop = {
577+
bindingProperties: {
578+
property: 'class', // 'class' is a reserved keyword
579+
},
580+
};
581+
582+
const result = buildBindingExpression(prop);
583+
584+
// Should be escaped as 'classProp'
585+
expect((result as any).text).toBe('classProp');
586+
});
587+
588+
it('should sanitize invalid characters in property names', () => {
589+
const prop = {
590+
bindingProperties: {
591+
property: 'user@name!',
592+
},
593+
};
594+
595+
const result = buildBindingExpression(prop);
596+
597+
// Should be sanitized
598+
expect((result as any).text).toBe('user@name!');
599+
});
600+
601+
it('should generate snapshot for simple property', () => {
602+
const prop = {
603+
bindingProperties: {
604+
property: 'simpleProperty',
605+
},
606+
};
607+
608+
assertASTMatchesSnapshot(buildBindingExpression(prop));
609+
});
610+
611+
it('should generate snapshot for property with field', () => {
612+
const prop = {
613+
bindingProperties: {
614+
property: 'user',
615+
field: 'address',
616+
},
617+
};
618+
619+
assertASTMatchesSnapshot(buildBindingExpression(prop));
620+
});
621+
622+
it('should generate snapshot for property with nested field access', () => {
623+
const prop = {
624+
bindingProperties: {
625+
property: 'data',
626+
field: 'nested.field',
627+
},
628+
};
629+
630+
assertASTMatchesSnapshot(buildBindingExpression(prop));
631+
});
632+
});
360633
});

‎packages/codegen-ui-react/lib/react-component-render-helper.ts

Lines changed: 103 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ import { getChildPropMappingForComponentName } from './workflow/utils';
6565
import nameReplacements from './name-replacements';
6666
import keywords from './keywords';
6767
import { buildAccessChain } from './forms/form-renderer-helper/form-state';
68+
import { scriptingPatterns } from './utils/constants';
6869

6970
export function getFixedComponentPropValueExpression(prop: FixedStudioComponentProperty): StringLiteral {
7071
return factory.createStringLiteral(prop.value.toString(), true);
@@ -128,21 +129,113 @@ export function isActionEvent(event: StudioComponentEvent): event is ActionStudi
128129
}
129130

130131
/**
131-
* case: has field => <prop.bindingProperties.property>?.<prop.bindingProperties.field>
132-
* case: no field => <prop.bindingProperties.property>
132+
* This function checks for various potentially malicious patterns including:
133+
* - Dangerous JavaScript functions (eval, Function constructor, setTimeout, etc.)
134+
* - DOM manipulation attempts
135+
* - Event handler injections
136+
* - Dangerous URL protocols
137+
* - Script tag variations
138+
* - SVG exploit attempts
139+
* - Prototype pollution patterns
140+
* - Template literal injection attempts
141+
*
142+
* @param {string} str - The input string to validate
143+
* @returns {string} Returns an empty string if dangerous patterns are found, otherwise returns the trimmed input string
144+
*
145+
* @example
146+
* // Safe string
147+
* filterScriptingPatterns("Hello World"); // returns "Hello World"
148+
*
149+
* // Dangerous string
150+
* filterScriptingPatterns("eval('alert(1)')"); // returns ""
151+
*/
152+
export function filterScriptingPatterns(str: string): string {
153+
if (!str) return '';
154+
155+
// Check for dangerous JavaScript patterns
156+
if (scriptingPatterns.some((pattern) => pattern.test(str))) {
157+
return '';
158+
}
159+
160+
return str.trim();
161+
}
162+
163+
/**
164+
* Sanitizes and validates property values to ensure they are safe JavaScript identifiers.
165+
*
166+
* @param {string} propertyValue - The property value to be escaped/sanitized
167+
* @returns {string} A safe JavaScript identifier:
168+
* - Returns "{propertyValue}Prop" if the value is a reserved keyword
169+
* - Returns sanitized value after filtering scripting patterns
170+
*
171+
* @example
172+
* // Reserved keyword example
173+
* escapePropertyValue('class') // returns 'classProp'
174+
*
175+
* // Normal property
176+
* escapePropertyValue('userName') // returns 'userName'
177+
*/
178+
export function escapePropertyValue(propertyValue: string): string {
179+
// First check if it's a reserved keyword
180+
if (keywords.has(propertyValue)) {
181+
return `${propertyValue}Prop`;
182+
}
183+
184+
// Then sanitize the propery value to ensure it's a valid JavaScript identifier
185+
return filterScriptingPatterns(propertyValue);
186+
}
187+
188+
/**
189+
* Builds a TypeScript expression for property binding in the
190+
* UI component generation proces.
191+
*
192+
* Creates either:
193+
* 1. An identifier
194+
* 2. An optional chained property access
195+
*
196+
* @param {BoundStudioComponentProperty} prop - The bound property configuration object
197+
*
198+
* @returns {Expression} Returns one of:
199+
* - Identifier: When no field is specified
200+
* - PropertyAccessChain: When accessing a nested field with optional chaining
201+
*
202+
* {
203+
* "componentType": "Button",
204+
* "name": "MyButton",
205+
* "properties": {
206+
* "disabled": {
207+
* "bindingProperties": {
208+
* "property": "eval('if(!window.x){alert(document.domain);window.x=1}')"
209+
* }
210+
* }
211+
* }
212+
* }
213+
*
214+
* @see {factory} NodeFactory https://github.com/microsoft/TypeScript/blob/main/src/compiler/factory/nodeFactory.ts
215+
* - createIdentifer
216+
* https://github.com/microsoft/TypeScript/blob/main/src/compiler/factory/nodeFactory.ts#L1325
217+
* - createPropertyAccessChain
218+
* https://github.com/microsoft/TypeScript/blob/main/src/compiler/factory/nodeFactory.ts#L2929
133219
*/
134220
export function buildBindingExpression(prop: BoundStudioComponentProperty): Expression {
135221
const {
136222
bindingProperties: { property },
137223
} = prop;
138-
const identifier = factory.createIdentifier(keywords.has(property) ? `${property}Prop` : property);
139-
return prop.bindingProperties.field === undefined
140-
? identifier
141-
: factory.createPropertyAccessChain(
142-
identifier,
143-
factory.createToken(SyntaxKind.QuestionDotToken),
144-
prop.bindingProperties.field,
145-
);
224+
225+
const escapedPropertyValue = escapePropertyValue(property);
226+
const identifier = factory.createIdentifier(escapedPropertyValue);
227+
228+
if (prop.bindingProperties.field === undefined) {
229+
return identifier;
230+
}
231+
232+
const propertyAccess = factory.createPropertyAccessChain(
233+
identifier,
234+
factory.createToken(SyntaxKind.QuestionDotToken),
235+
prop.bindingProperties.field,
236+
);
237+
238+
return propertyAccess;
146239
}
147240

148241
export function buildBindingAttr(prop: BoundStudioComponentProperty, propName: string): JsxAttribute {

‎packages/codegen-ui-react/lib/utils/constants.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,47 @@ export const STORAGE_FILE_ALGO_TYPE = 'SHA-1';
2222
export const AMPLIFY_JS_V5 = '5.0.0';
2323

2424
export const AMPLIFY_JS_V6 = '6.0.0';
25+
26+
export const scriptingPatterns = [
27+
// JavaScript functions
28+
/eval\s*\(/i,
29+
/Function\s*\(/i,
30+
/setTimeout\s*\(/i,
31+
/setInterval\s*\(/i,
32+
/new\s+Function/i,
33+
/import\s*\(/i,
34+
/require\s*\(/i,
35+
36+
// DOM manipulation
37+
/document\./i,
38+
/window\./i,
39+
/location\./i,
40+
41+
// Event handlers
42+
/on\w+\s*=/i, // matches onerror=, onload=, etc.
43+
44+
// Dangerous protocols
45+
/javascript:/i,
46+
/data:/i,
47+
/vbscript:/i,
48+
49+
// Script tags and variations
50+
/<script/i,
51+
/<\/script/i,
52+
/<x:script/i,
53+
54+
// SVG exploits
55+
/<svg/i,
56+
/xlink:href/i,
57+
58+
// Object prototype attacks
59+
/\[\s*Symbol\s*\./i,
60+
/__proto__/i,
61+
/prototype\s*\./i,
62+
63+
// Template literal attacks
64+
/\$\{/i,
65+
66+
// Base64 indicators
67+
/base64/i,
68+
];

0 commit comments

Comments
 (0)
Please sign in to comment.