Skip to content

C4: UpdateElementStyle colours are not validated before reaching a CSS declaration #8057

Description

@filipsajdak

C4's UpdateElementStyle colours ($bgColor, $borderColor, $fontColor) are stored and interpolated into a CSS declaration list without any validation. A value containing ; corrupts the declaration it lands in, and under look: handDrawn an arbitrary declaration of the author's choosing reaches the element's style attribute.

Filing this as input validation rather than a vulnerability - see the severity section, which I have tried to keep honest rather than alarming. It is the element-colour counterpart of the asColor guard added for relationship colours in #7883.

Where the value is stored unchecked

packages/mermaid/src/diagrams/c4/c4Db.ts:504-527, updateElStyle:

if (fontColor !== undefined && fontColor !== null) {
  if (typeof fontColor === 'object') {
    const [key, value] = Object.entries(fontColor)[0];
    old[key] = value;
  } else {
    old.fontColor = fontColor;
  }
}

No sanitizeText, no colour check. The grammar preserves the value intact: parser/c4Diagram.jison:176 accepts [^"]+ for a STR_VALUE, so ; and : survive; only .trim() is applied. updateRelStyle (:581-608) is the same, so the db layer treats elements and relationships alike.

Worth noting for contrast: text is validated - sanitizeText for the title and diagram type in the db (:69, :771), and for labels at render (c4LabelHelper.ts:59). Colours are validated nowhere.

What actually happens - two separate effects

The value is interpolated into one entry of a declaration array at c4ShapeAdapter.ts:148-154:

styles.push(`color:${shape.fontColor ?? '#FFFFFF'}`);

1. Default look: the declaration is corrupted, not extended. styles2Map (rendering-util/rendering-elements/shapes/handDrawnShapeStyles.ts:44-51) does const [key, value] = style.split(':') - no limit, and only the first two elements are read, so everything after the second colon is dropped. $fontColor="red;font-family:x" becomes:

color:red;font-family !important

color:red applies but loses its !important, and font-family has no colon so the browser discards it. Nothing attacker-chosen lands. It is a silent-corruption bug rather than an injection: a value with a ; in it quietly changes what the rest of the declaration means.

2. look: handDrawn: a complete declaration does land. That path passes the raw cssStyles array straight to the attribute, bypassing styles2Map entirely - drawRect.ts:49, person.ts:52-53, cylinder.ts:105-106:

rect.attr('class', 'basic label-container').attr('style', handleUndefinedAttr(cssStyles));

d3 stringifies the array comma-separated, so a payload ending in ; detaches cleanly from the following comma. $bgColor="red;background-image:url(https://example.invalid/x);" produces a style attribute in which fill:red and background-image:url(https://example.invalid/x) are both valid declarations, with only the trailing ,color:#FFFFFF,... discarded.

look is settable from diagram frontmatter - it is not in the secure-keys list (schemas/config.schema.yaml:263-271) and is read at c4Renderer.ts:267.

Affects all four shapes C4 resolves to: rounded/fr-rect (drawRect), person, cylinder, h-cyl.

Incidentally, that same raw-array .attr('style', cssStyles) is a pre-existing bug independent of any injection: a comma-separated array is not valid CSS, so in the handDrawn look those inline styles are mostly discarded anyway.

Severity: low

Stated plainly so nobody over- or under-reacts:

  • The values reach the DOM only via .attr('style', ...) on SVG elements - never innerHTML, never a <style> element, never a selector context. So no attribute-selector exfiltration and no rules that can target other elements.
  • C4 element labels are SVG <text>/<tspan>, not HTML: c4ShapeAdapter.ts:194 sets useHtmlLabels: false, and every C4 node carries a stereotype, so shapes/util.ts:20 always routes to c4LabelHelper, which hardcodes it false too.
  • No script execution. Nothing evaluates CSS here.

The realistic worst case is a diagram that renders wrongly, plus a possible external fetch under handDrawn if a url(...) declaration lands. I have not verified in a browser whether such a fetch actually occurs - background properties do not paint on SVG shapes, so it may well not - and I would rather say so than assert it. If it does, it is adjacent to #7645.

Suggested fix

Validate the three colours before they reach a declaration, the same way #7883 does for relationship colours (asColor, on feature/c4-migrate-edges):

const accepted =
  typeof globalThis.CSS?.supports === 'function'
    ? globalThis.CSS.supports('color', value)
    : /^(#[\da-f]{3,8}|[a-z]+|rgba?\([\d\s%,./]+\)|hsla?\([\d\s%,./deg]+\))$/i.test(value);

CSS.supports('color', value) rejects anything that is not a colour standing alone, with a conservative pattern fallback for jsdom. A shared helper used by both the element and relationship paths would be the tidiest outcome, since #7883 and #8042 both already carry a copy.

Two independent hardening steps also worth considering on their own merits:

  • give styles2Map a split limit so a stray ; cannot silently redefine a declaration, rather than relying on truncation;
  • fix the handDrawn .attr('style', cssStyles) calls to join the array, which removes the bypass and incidentally makes those inline styles work at all.

Happy to send a PR for the C4-side validation; the two rendering-util items touch shared code and affect every diagram, so they want a maintainer's view on scope first.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Status: TriageNeeds to be verified, categorized, etc

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions