Skip to content

[Security] Fix chat template resource-exhaustion DoS (GHSA-4hhp-h66f-… - #50300

Merged
vllm-bot merged 5 commits into
vllm-project:mainfrom
jperezdealgaba:fix/chat-template-resource-exhaustion-dos
Sep 30, 2026
Merged

vllm-bot merged 5 commits into
vllm-project:mainfrom
jperezdealgaba:fix/chat-template-resource-exhaustion-dos

Conversation

@jperezdealgaba

Copy link
Copy Markdown
Contributor

…j5j7)

Block unbounded Jinja2 template rendering that allows an authenticated API client to permanently wedge all chat-style endpoints via CPU or memory exhaustion.

Python frontend:

  • Reject user-supplied chat_template in /v1/responses chat_template_kwargs (closes the GHSA-jmjw-q4x3-hmc2 gate omission)
  • Add static AST validation rejecting dangerous patterns (excessive range(), string multiplication, deeply nested loops) before rendering
  • Add configurable async render timeout (VLLM_CHAT_TEMPLATE_RENDER_TIMEOUT)

Rust frontend:

  • Enable minijinja fuel feature and set 500K instruction budget per render
  • Wrap synchronous render() in tokio::task::spawn_blocking to avoid stalling the shared async runtime

@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.

@DarkLight1337 DarkLight1337 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this needed when OnlineRenderer.validate_chat_template already exists?

@jperezdealgaba

Copy link
Copy Markdown
Contributor Author

@DarkLight1337 Wasn't aware of that. Let me take a look

@jperezdealgaba
jperezdealgaba force-pushed the fix/chat-template-resource-exhaustion-dos branch 2 times, most recently from eaafcea to c6c1c9a Compare July 29, 2026 17:28
@jperezdealgaba

Copy link
Copy Markdown
Contributor Author

@DarkLight1337 I simplified the MR to only address this issue. More hardening can be done in future MRs

Comment thread vllm/renderers/hf.py Outdated
/// Default fuel budget for template rendering. Prevents unbounded loops and
/// string operations from exhausting CPU. Each template instruction consumes
/// one unit of fuel; when depleted, rendering aborts with an error.
const DEFAULT_TEMPLATE_FUEL: u64 = 500_000;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

PTAL @BugenZhao

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c6c1c9a498

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread vllm/renderers/hf.py Outdated
Comment thread vllm/entrypoints/openai/responses/serving.py Outdated
@jperezdealgaba
jperezdealgaba force-pushed the fix/chat-template-resource-exhaustion-dos branch 2 times, most recently from 388473b to 7b316b2 Compare July 30, 2026 07:10
Comment thread vllm/renderers/hf.py Outdated
Comment thread vllm/renderers/hf.py Outdated
@jperezdealgaba
jperezdealgaba force-pushed the fix/chat-template-resource-exhaustion-dos branch from 7b316b2 to d0fed91 Compare July 30, 2026 07:25

@DarkLight1337 DarkLight1337 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Python side LGTM, I'll leave Rust frontend to @BugenZhao

@mergify

mergify Bot commented Aug 5, 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

@TheZ3ro

TheZ3ro commented Sep 7, 2026

Copy link
Copy Markdown

I was testing this security fix and noticed that after a resource-exhaustion request, the ThreadPoolExecutor is not correctly replaced and instead cause an even worse DoS state where every subsequent legit request (completions, tokenize, etc) beside the /health one is met with a 500 cannot schedule new futures after shutdown error.

This is because by the new executor do not schedule the async func that are declared for a BaseRenderer. Those are still bound to the old shutdown executor instead.

Since health return 200 this also prevents further recovery of the service.

@jperezdealgaba
jperezdealgaba force-pushed the fix/chat-template-resource-exhaustion-dos branch from 3696c88 to c179575 Compare September 10, 2026 13:54
@jperezdealgaba

Copy link
Copy Markdown
Contributor Author

@TheZ3ro Can you check again? It should be fixed

@mergify mergify Bot removed the needs-rebase label Sep 10, 2026
@TheZ3ro

TheZ3ro commented Sep 14, 2026

Copy link
Copy Markdown

@jperezdealgaba I confirm that I cannot reproduce anymore

@mergify

mergify Bot commented Sep 18, 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

@mergify mergify Bot added the needs-rebase label Sep 18, 2026
@TheZ3ro

TheZ3ro commented Sep 30, 2026

Copy link
Copy Markdown

any news on this? when will the fix land?

jperezdealgaba and others added 3 commits September 30, 2026 13:40
…j5j7)

Block unbounded Jinja2 template rendering that allows an authenticated
API client to permanently wedge all chat-style endpoints via CPU or
memory exhaustion.

Python frontend:
- Reject user-supplied chat_template in /v1/responses chat_template_kwargs
  (closes the GHSA-jmjw-q4x3-hmc2 gate omission)
- Add static AST validation rejecting dangerous patterns (excessive
  range(), string multiplication, deeply nested loops) before rendering
- Add configurable async render timeout (VLLM_CHAT_TEMPLATE_RENDER_TIMEOUT)

Rust frontend:
- Enable minijinja fuel feature and set 500K instruction budget per render
- Wrap synchronous render() in tokio::task::spawn_blocking to avoid
  stalling the shared async runtime

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

Signed-off-by: jperezde <jperezde@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…imeout

Replacing self._executor left tokenize, decode, pooling, and derender
make_async closures bound to the shutdown pool, so later requests failed
with cannot schedule new futures after shutdown.

Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
Co-authored-by: Cursor Grok 4.6
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
@jperezdealgaba
jperezdealgaba force-pushed the fix/chat-template-resource-exhaustion-dos branch from c179575 to b820f42 Compare September 30, 2026 11:40
@jperezdealgaba

Copy link
Copy Markdown
Contributor Author

@TheZ3ro I fixed the MR conflicts. The MR needs to be reviewed by the team

@mergify mergify Bot removed the needs-rebase label Sep 30, 2026

@DarkLight1337 DarkLight1337 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's just merge this, seems @BuganZhao is busy

@DarkLight1337

Copy link
Copy Markdown
Member

/ci run

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) September 30, 2026 11:58
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 30, 2026
@github-actions

Copy link
Copy Markdown

❌ This PR is 1 commit behind upstream main. Your branch must contain every commit currently on upstream main. No new CI build was started. Merge or rebase onto the latest main, then rerun /ci run. To test this branch at your own risk, use /ci run --allow-stale.

@jperezdealgaba

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #92127 for commit 24d57133e0cc.

@jperezdealgaba

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

Copy link
Copy Markdown

✅ Queued 1 failed job(s) for retry in Buildkite CI #92127.

@vllm-bot
vllm-bot merged commit cff08b4 into vllm-project:main Sep 30, 2026
11 checks passed
vrdn-23 added a commit to vrdn-23/vllm that referenced this pull request Oct 8, 2026
…nflicts

Took the branch's vllm/envs.py wholesale: main's legacy TYPE_CHECKING block
and environment_variables dict are already superseded by the pydantic
*Settings models here. Then ported main's semantic delta from the 57
main-side commits that touched vllm/envs.py since 03a8d0b:

  vllm-project#52431 VLLM_USE_MODELSCOPE: description (bool already accepts "1")
  vllm-project#52216 prefix_cache_retention_interval promoted to an arg (see vllm-project#55353)
  vllm-project#48484 + VLLM_REPLICATE_EMBED
  vllm-project#52633 + VLLM_CPU_CI_ENV
  vllm-project#52182 - VLLM_TEST_FORCE_FP8_MARLIN
  vllm-project#52523 VLLM_API_KEY not a compile factor (branch already marked it)
  vllm-project#52998 VLLM_ALLREDUCE_USE_FLASHINFER default -> True
  vllm-project#53040 + VLLM_DISABLE_DSV4_MEGAMOE_SHARED_EXPERT_FUSION
  vllm-project#53053 + VLLM_KIMI_K3_GEMM_AR
  vllm-project#50501 + VLLM_XPU_INC_WNA16_BACKEND (case-sensitive Literal)
  vllm-project#52018 + VLLM_B12X_MOE_FP4_FORCE_A16
  vllm-project#43018 + VLLM_KV_OFFLOAD_MAX_BATCH_DESCRIPTORS
  vllm-project#51718 VLLM_KV_CACHE_LAYOUT Literal widened to 8 layouts + description
  vllm-project#51896 + VLLM_MAX_MEDIA_DOWNLOAD_SIZE_MB
  vllm-project#53219 VLLM_ENABLE_COHERE_API: description
  vllm-project#53559 - VLLM_TRITON_ATTN_USE_TD shim (already absent on the branch)
  vllm-project#53561 VLLM_MAX_AUDIO_CLIP_FILESIZE_MB: description
  vllm-project#53378 VLLM_ELASTIC_EP_SCALE_UP_LAUNCH not a compile factor
  vllm-project#53141 VLLM_ROCM_USE_AITER_FP4_ASM_GEMM removed; restored by vllm-project#57055
  vllm-project#54160 + VLLM_ENABLE_HPC_OPS
  vllm-project#53536 + VLLM_XPU_FORCE_N_CONTIG_WEIGHT
  vllm-project#53576 + VLLM_ALLREDUCE_USE_FLASHINFER_PCIE_IPC
  vllm-project#54632 + VLLM_MAX_EMBED_DECODE_BYTES (not a compile factor)
  vllm-project#54579 + VLLM_ENABLE_SCALE_OUT_ENDPOINTS; removed by vllm-project#55176 (net zero)
  vllm-project#53491 VLLM_GPU_SYNC_CHECK: description
  vllm-project#54371 + VLLM_PLE_CPU_OFFLOAD; reworked vllm-project#56512, dropped vllm-project#57937 (net zero)
  vllm-project#48247 VLLM_ROCM_USE_AITER_CUSTOM_AR: description
  vllm-project#55353 - VLLM_MM_HASHER_ALGORITHM (+ _lower_mm_hasher),
         - VLLM_PREFIX_CACHE_RETENTION_INTERVAL
  vllm-project#54985 - VLLM_NIXL_EP_MAX_NUM_RANKS
  vllm-project#56512 VLLM_PLE_CPU_OFFLOAD rework (net zero, see vllm-project#54371)
  vllm-project#55176 - VLLM_ENABLE_SCALE_OUT_ENDPOINTS (net zero, see vllm-project#54579)
  vllm-project#53940 + VLLM_ROCM_USE_AITER_MOE_SITUV2 (case-insensitive Literal)
  vllm-project#41074 + VLLM_SKIP_VERSION_SUFFIX (not a compile factor)
  vllm-project#48599 - envs.validate_environ (moved to Platform.validate_environ,
         which reads the branch's environment_variables shim)
  vllm-project#52136 pydocstyle docstring restyle of legacy helpers (no-op here)
  vllm-project#57212 docstring rejoin of legacy helpers (no-op here)
  vllm-project#56913 + FORCE_COLOR (alias, non-"0" is True, not a compile factor)
  vllm-project#57055 VLLM_ROCM_USE_AITER_FP4_ASM_GEMM restored (net zero, see vllm-project#53141)
  vllm-project#57142 shm buffer name autogen -> new _generate_shm_name() (<=30 chars)
  vllm-project#56685 VLLM_HUMMING_INPUT_QUANT_CONFIG: description
  vllm-project#57937 - VLLM_PLE_CPU_OFFLOAD (net zero, see vllm-project#54371)
  vllm-project#50814 + VLLM_MXFP4_EMULATION_DEQUANT_AT_LOAD
  vllm-project#57428 + VLLM_ENABLE_GEMM_RS, - VLLM_KIMI_K3_GEMM_RS
  vllm-project#51600 - VLLM_XPU_ENABLE_XPU_GRAPH
  vllm-project#58582 + VLLM_TRITON_JIT_WARMUP_NUM_THREADS (not a compile factor)
  vllm-project#55145 + VLLM_XPU_PP_MICROBATCH
  vllm-project#58201 VLLM_ROCM_USE_AITER_MOE_SITUV2 6-way choice,
         - VLLM_ROCM_USE_AITER_MOE_SITUV2_A8W4
  vllm-project#58919 + VLLM_MOONCAKE_CONNECTOR_TIMEOUT
  vllm-project#56861 + VLLM_ROCM_AITER_MLA_DCP_VERIFY (case-insensitive Literal)
  vllm-project#53492 + VLLM_ROCM_USE_AITER_TRITON_SPARSE_MLA
  vllm-project#57991 VLLM_DEEPEP_V2_ALLOW_HYBRID_MODE default True (branch already
         True) + description
  vllm-project#58819 + VLLM_ROCM_USE_AITER_MOE_A4W4_DSV4 (tri-state, bool(int(v)))
  vllm-project#50300 + VLLM_CHAT_TEMPLATE_RENDER_TIMEOUT
  vllm-project#54024 VLLM_CPU_INT4_W4A8: description
  vllm-project#59781 - VLLM_ENGINE_ITERATION_TIMEOUT_S
  vllm-project#57995 + VLLM_FLASHINFER_MOE_A2A_LOW_PRECISION_COMBINE
  vllm-project#59965 VLLM_ROCM_AITER_MLA_DCP_VERIFY default route (final "auto")

Also VLLM_SSM_CONV_STATE_LAYOUT's description follows vllm-project#51718's layout rename.

Parity: 311 branch fields vs 311 main runtime entries, no difference in
either direction. Every var's value matches main's lambdas in an empty
environment and under 15 override scenarios, except case normalization of
the two lowercased ROCm choices (both callers already call .lower()).

Intentionally not ported: main's new consumer tests that match
env_with_choices' error text (test_invalid_backend_value_raises,
test_dcp_verify_env_rejects_unknown_route, and the ROCm-gated
test_aiter_moe_situv2_rejects_unknown_activation) fail on this branch:
pydantic's ValidationError names the field, not the env var. Invalid values
are still rejected. Tracked as a branch-wide follow-up.

Other conflicts and merge fallout:
- requirements/docs.txt, requirements/test/{cpu,cuda,rocm,xpu}.txt: took
  main's lockfiles and regenerated them with the pip-compile pre-commit
  hooks; they differ from main only by the branch's pydantic-settings lines.
- tests/test_envs_pydantic.py: dropped the mm-hasher test, pointed the
  validate_environ tests at Platform.validate_environ, updated the shm-name
  prefix assertion.
- check-test-tethering (new on main): wired test_envs_pydantic.py into the
  CPU params/env job; allowlisted tests/docs/test_env_vars_gen.py, which
  cannot run from a CI job's tests/ cwd (tests/docs shadows docs/).

Tests: pytest tests/test_envs.py tests/test_envs_pydantic.py
tests/docs/test_env_vars_gen.py -> 55 passed; pre-commit run --files on all
changed files -> clean.

AI assistance (Claude) was used for this merge resolution.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Vinay Damodaran <vrdn@hey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend ready ONLY add when PR is ready to merge/full CI is needed rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants