T09 · Insecure Skill Coding Practices
Error
- Location
- src/index.ts:252
- Finding
- SVG Markup Injection Through Unvalidated Custom Colors<![CDATA[ ## Vulnerability Details **File Location**: `src/index.ts:252`, `src/index.ts:270`, `src/index.ts:322`, `src/index.ts:360`, `src/index.ts:396`, `src/index.ts:432`, and `src/index.ts:573` **Vulnerability Type**: SVG markup injection **Risk Level**: High ### Vulnerable Code ```ts const palette = colors || DEFAULT_COLORS; ``` Caller-controlled palette entries are subsequently interpolated directly into SVG attributes. For example: ```ts const color = colors[s % colors.length]; svg += `<rect x="${x}" y="${y}" width="${barWidth}" height="${barH}" fill="${color}" rx="2"/>`; ``` The same unsafe interpolation pattern is used for chart bars, legends, line paths, points, and pie slices: ```ts const color = colors[s % colors.length]; svg += `<path d="${pathD}" fill="none" stroke="${color}" stroke-width="2.5" stroke-linejoin="round"/>`; ``` ```ts const color = colors[i % colors.length]; svg += `<path d="M${cx},${cy} L${x1.toFixed(2)},${y1.toFixed(2)} A${radius},${radius} 0 ${largeArc},1 ${x2.toFixed(2)},${y2.toFixed(2)} Z" fill="${color}" stroke="white" stroke-width="2"/>`; ``` ### Technical Analysis The `generate_chart` tool accepts a caller-controlled `colors` array but performs no validation or XML attribute escaping before inserting each color into `fill` or `stroke` attributes. Because the value is placed inside a double-quoted SVG attribute, an attacker can supply quotation marks and SVG markup that terminate the existing attribute or element and introduce additional elements. A conceptual malicious value can close the `fill` attribute and inject an active SVG element such as a script or event-bearing element. The existing `escapeXml` function is applied to chart titles, labels, and series names, but it is not applied to color values. Generic XML escaping would prevent attribute breakout, while strict color validation would additionally prevent unintended CSS or SVG constructs. The tool returns both raw SVG and a base64 SVG data URI. Rendering be ...[truncated 1560 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Validate every custom color against a strict allowlist before chart generation. If only hexadecimal colors are required, accept formats such as: ```ts const HEX_COLOR = /^#[0-9a-fA-F]{6}(?:[0-9a-fA-F]{2})?$/; function validateColors(colors: unknown): string[] { if (colors === undefined) return DEFAULT_COLORS; if (!Array.isArray(colors) || colors.length === 0) { throw new Error('colors must be a non-empty array'); } return colors.map(color => { if (typeof color !== 'string' || !HEX_COLOR.test(color)) { throw new Error('Each color must be a valid hexadecimal color'); } return color; }); } ``` 2. Use the validated palette in the handler: ```ts const palette = validateColors(colors); ``` 3. XML-escape every value inserted into an SVG attribute, even after semantic validation: ```ts const color = escapeXml(colors[s % colors.length]); ``` 4. Reject empty color arrays, which otherwise produce undefined palette entries. 5. Sanitize generated SVG using a well-maintained SVG sanitizer before rendering it in a browser. 6. Prefer rendering untrusted SVG through a non-scriptable image pipeline. If inline SVG is necessary, enforce a restrictive Content Security Policy and do not permit scripts, external resources, or event-handler attributes. 7. Add regression tests using color values containing `"`, `'`, `<`, `>`, event handlers, and SVG tags, and assert that these inputs are rejected rather than included in the output. ]]>