fix(background): canonicalize the heropattern colour before templating - #250
Merged
LeadcodeDev merged 1 commit intoSep 22, 2026
Merged
Conversation
53 tasks
LeadcodeDev
force-pushed
the
refactor/decorative-paint-path
branch
from
September 22, 2026 06:10
3dcfc14 to
afbca7b
Compare
LeadcodeDev
force-pushed
the
fix/heropattern-colour-escape
branch
from
September 22, 2026 06:11
11ed92d to
14f8d7e
Compare
LeadcodeDev
force-pushed
the
refactor/decorative-paint-path
branch
from
September 22, 2026 08:35
afbca7b to
6a2f28e
Compare
LeadcodeDev
force-pushed
the
fix/heropattern-colour-escape
branch
from
September 22, 2026 08:35
14f8d7e to
3163f12
Compare
LeadcodeDev
force-pushed
the
refactor/decorative-paint-path
branch
from
September 22, 2026 08:45
6a2f28e to
e5699b7
Compare
LeadcodeDev
force-pushed
the
fix/heropattern-colour-escape
branch
from
September 22, 2026 08:45
3163f12 to
c760058
Compare
LeadcodeDev
changed the base branch from
refactor/decorative-paint-path
to
chantier/audit-2026-09
September 22, 2026 08:53
LeadcodeDev
force-pushed
the
fix/heropattern-colour-escape
branch
from
September 22, 2026 09:01
c760058 to
1f3ac28
Compare
`cfg.color` is a free-form `String` on `HeropatternConfig` (crates/rustmotion-core/src/schema/background.rs:216, `#[serde(default = "default_hero_color")]`) with no validation anywhere — `validate_animated_bg` (crates/rustmotion/src/include.rs:267-273) checks only that the pattern *name* resolves. The template lands inside a double-quoted attribute (`fill="{{color}}"`), so a colour containing `"` closes the attribute and injects arbitrary markup into the document usvg then parses. usvg 0.46's `Options::default()` keeps the default `ImageHrefResolver`, whose string resolver I read at usvg-0.46.0/src/parser/image.rs:85-112: it calls `std::fs::read(&path)` on any href that `path.exists()` accepts — with `resources_dir: None` an absolute path resolves to itself — so an injected `<image href="/absolute/path.png">` is decoded and painted into the frame. The impact is bounded (a scenario can already read a local image legitimately via `{"type":"image","src":"/abs/path.png"}`, and there is no network resolver), so this is a hardening gap rather than a privilege escalation. The quieter half is availability: any malformed splice makes `Tree::from_data` fail and line 601 `return`s, so the background silently does not render with no diagnostic at all.
Refs #220
LeadcodeDev
force-pushed
the
fix/heropattern-colour-escape
branch
from
September 22, 2026 09:05
1f3ac28 to
a707004
Compare
LeadcodeDev
added a commit
that referenced
this pull request
Sep 22, 2026
#250) `cfg.color` is a free-form `String` on `HeropatternConfig` (crates/rustmotion-core/src/schema/background.rs:216, `#[serde(default = "default_hero_color")]`) with no validation anywhere — `validate_animated_bg` (crates/rustmotion/src/include.rs:267-273) checks only that the pattern *name* resolves. The template lands inside a double-quoted attribute (`fill="{{color}}"`), so a colour containing `"` closes the attribute and injects arbitrary markup into the document usvg then parses. usvg 0.46's `Options::default()` keeps the default `ImageHrefResolver`, whose string resolver I read at usvg-0.46.0/src/parser/image.rs:85-112: it calls `std::fs::read(&path)` on any href that `path.exists()` accepts — with `resources_dir: None` an absolute path resolves to itself — so an injected `<image href="/absolute/path.png">` is decoded and painted into the frame. The impact is bounded (a scenario can already read a local image legitimately via `{"type":"image","src":"/abs/path.png"}`, and there is no network resolver), so this is a hardening gap rather than a privilege escalation. The quieter half is availability: any malformed splice makes `Tree::from_data` fail and line 601 `return`s, so the background silently does not render with no diagnostic at all. Refs #220
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Severity Low, category security. Location:
crates/rustmotion/src/engine/render/background.rs:595Impact
cfg.coloris a free-formStringonHeropatternConfig(crates/rustmotion-core/src/schema/background.rs:216,#[serde(default = "default_hero_color")]) with no validation anywhere —validate_animated_bg(crates/rustmotion/src/include.rs:267-273) checks only that the pattern name resolves. The template lands inside a double-quoted attribute (fill="{{color}}"), so a colour containing"closes the attribute and injects arbitrary markup into the document usvg then parses. usvg 0.46'sOptions::default()keeps the defaultImageHrefResolver, whose string resolver I read at usvg-0.46.0/src/parser/image.rs:85-112: it callsstd::fs::read(&path)on any href thatpath.exists()accepts — withresources_dir: Nonean absolute path resolves to itself — so an injected<image href="/absolute/path.png">is decoded and painted into the frame. The impact is bounded (a scenario can already read a local image legitimately via{"type":"image","src":"/abs/path.png"}, and there is no network resolver), so this is a hardening gap rather than a privilege escalation. The quieter half is availability: any malformed splice makesTree::from_datafail and line 601returns, so the background silently does not render with no diagnostic at all.Fix
XML-escape
cfg.colorbefore substitution (at minimum reject or strip",<,&), or better, parse it with the existingrustmotion_core::engine::renderer::color4f_from_hex/paint_from_hexand re-emit a canonical#rrggbb. Setusvg::Options { image_href_resolver: ImageHrefResolver { resolve_string: Box::new(|_, _| None), .. }, .. }since a heropattern never legitimately references an external image. Replace the silentreturnat 601-603 with a warning naming the pattern and the colour.Evidence the audit read
Part of the September 2026 audit remediation chantier. Refs #220 (RM-45).