Skip to content

fix(green-screen): validate and sanitize bg_color to prevent filter injection - #716

Merged
calesthio merged 1 commit into
OpenMontageInc:mainfrom
Huge:fix/chromakey-bg-color-injection
Nov 8, 2026
Merged

calesthio merged 1 commit into
OpenMontageInc:mainfrom
Huge:fix/chromakey-bg-color-injection

Conversation

@Huge

@Huge Huge commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Validates and sanitizes bg_color in GreenScreenProcessor (tools/video/green_screen_processor.py) to prevent FFmpeg filter graph injection.

Previously, bg_color was only stripped of # (bg_color.lstrip("#")) and directly interpolated into the filter graph string in _process_chromakey():
f"color=c=0x{bg_hex}:size={width}x{height}"
If an attacker supplied a crafted bg_color string containing filter graph delimiters (such as commas, colons, or filter expressions like 000000,drawbox=...), arbitrary FFmpeg filter chains could be injected.

Changes

  • Added _validate_hex_color(bg_color: str) -> str to enforce strict 3, 6, or 8 character hexadecimal formatting and expansion (e.g. 000 -> 000000).
  • Validated bg_color upfront in execute() so invalid color inputs return a clean ToolResult(success=False, error=...) without launching subprocesses.
  • Applied validation in _process_chromakey() and _process_rembg() before passing into FFmpeg filter graph definitions or slicing RGB channels.
  • Added comprehensive unit tests in tests/tools/test_green_screen_chromakey.py covering valid colors and rejecting filter injection payloads / malformed hex values.

Testing

  • Ran focused unit tests:
    uv run --with pytest pytest tests/tools/test_green_screen_chromakey.py (18 passed)
  • Ran contract test suite:
    uv run --with-requirements requirements-dev.txt --with filelock pytest tests/contracts/ (1229 passed, 7 skipped)

Checklist

  • The change is focused on a single logical concern.
  • I ran the relevant tests locally (make test-contracts / make test) where applicable.
  • I updated docs/README if behavior or usage changed.
  • No unrelated files (build artifacts, local config) are included in the diff.

Disclaimer: Vulnerability spotted while test-driving BudgetScan https://huge.github.io/nano-analyzer static analysis. Fix human-verified.

…njection

Validate bg_color hex format in GreenScreenProcessor before
interpolating into FFmpeg filter graph commands in _process_chromakey
or parsing RGB channels in _process_rembg. Rejects malformed strings
and filter graph delimiter injections.

Reported-by: BS.ehlas.cz
@Huge

Huge commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Gentle ping on this one: it validates bg_color before the green-screen tool puts it into the FFmpeg filter graph. Is anything missing before it can be reviewed?

@calesthio calesthio left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for validating background colors before filter construction. Reviewed the complete diff against main 63d1087a: accepted 3/6/8-digit hex forms are normalized, invalid/filter syntax is rejected before subprocess launch, and both background-removal paths use the validation. Validation: isolated offline suite — 18 passed, including native FFmpeg chroma-key rendering.

@calesthio calesthio closed this Nov 7, 2026
@calesthio calesthio reopened this Nov 7, 2026
calesthio pushed a commit that referenced this pull request Nov 8, 2026
@calesthio
calesthio merged commit 8538235 into OpenMontageInc:main Nov 8, 2026
1 check passed
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.

2 participants