Skip to content

[Security] Enforce server-side num_frames ceiling in VideoMediaIO merge - #51969

Open
jperezdealgaba wants to merge 2 commits into
vllm-project:mainfrom
jperezdealgaba:fix/num-frames-ceiling-bypass
Open

jperezdealgaba wants to merge 2 commits into
vllm-project:mainfrom
jperezdealgaba:fix/num-frames-ceiling-bypass

Conversation

@jperezdealgaba

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes the incomplete CVE-2026-34755 remediation reported in GHSA-vxqj-p4gw-9h4c: request-level media_io_kwargs.video.num_frames=-1 (or an arbitrarily large positive value) could override the engine frame-count ceiling, restoring the unbounded video/jpeg decode path that PR (security) Enforce frame limit in VideoMediaIO #38636 was meant to close.
  • Adds server-enforced clamping in VideoMediaIO.merge_kwargs: when runtime_kwargs are present, the merged num_frames is clamped to the engine ceiling (positive engine value → hard max; engine -1 → operator unlimited; absent → constructor default of 32). Requests may lower but never raise the ceiling.
  • Covers the fps-only override path where the existing fps/num_frames mutual-exclusion logic could pop the engine ceiling away, leaving the effective value unguarded.

Test plan

  • TestMergeKwargsNumFramesCeiling: 10 merge-policy unit tests covering clamp of -1, oversized, lower-allowed, engine unlimited, absent/empty defaults, fps-only wipe, and no-runtime passthrough.
  • test_merge_then_load_base64_enforces_ceiling: integration test through merge_media_io_kwargsVideoMediaIO.load_base64("video/jpeg", ...) confirming frame count is bounded.
  • All 46 tests in tests/multimodal/media/test_video.py pass (including pre-existing GPU-backend and decode tests).
  • All pre-commit hooks pass (ruff, mypy, typos, etc.).
.venv/bin/python -m pytest tests/multimodal/media/test_video.py -v
# 46 passed in 6.87s

Made with Cursor

Request-level media_io_kwargs could override the engine num_frames
setting with -1 or an arbitrarily large value, bypassing the frame-count
protection added by PR vllm-project#38636 for CVE-2026-34755. This restores
request-controlled, linearly scalable memory allocation at the video/jpeg
decoder.

Clamp the merged num_frames to the engine ceiling inside
VideoMediaIO.merge_kwargs so that requests can only lower, never raise or
disable, the operator limit. Engine num_frames=-1 (operator unlimited)
is respected; absent engine config falls back to the constructor default
of 32.

Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

Signed-off-by: jperezde <jperezde@redhat.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the multi-modality Related to multi-modality (#4194) label Aug 12, 2026
@DarkLight1337
DarkLight1337 requested a review from Isotr0py August 13, 2026 06:03
@m4rgh

m4rgh commented Aug 19, 2026

Copy link
Copy Markdown

Friendly ping @Isotr0py — when you have a chance, could you take a look at this PR? Thanks!

@mergify

mergify Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @jperezdealgaba.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-modality Related to multi-modality (#4194) needs-rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants