Skip to content

fix(dingtalk,api): validate webhook URL origin, cap cache, reject header injection - #6928

Closed
aaronlab wants to merge 1 commit into
NousResearch:mainfrom
aaronlab:fix/dingtalk-ssrf-and-api-header-injection
Closed

fix(dingtalk,api): validate webhook URL origin, cap cache, reject header injection#6928
aaronlab wants to merge 1 commit into
NousResearch:mainfrom
aaronlab:fix/dingtalk-ssrf-and-api-header-injection

Conversation

@aaronlab

Copy link
Copy Markdown
Contributor

Summary

  • dingtalk.py — SSRF via session_webhook: The session_webhook URL from incoming DingTalk messages is stored and later POSTed to (line 290) without any origin validation. An attacker controlling the DingTalk message can set session_webhook to http://169.254.169.254/latest/meta-data/ (cloud metadata), http://127.0.0.1:6379 (Redis), or any internal service. Fix: add a regex check that only accepts the official https://api.dingtalk.com/ origin.

  • dingtalk.py — unbounded memory growth: _session_webhooks dict grows without limit — every unique chat_id creates a permanent entry, cleared only on disconnect(). A long-running gateway instance (months) accumulates millions of entries → OOM. Fix: cap at 500 entries with FIFO eviction (consistent with the dedup pattern at lines 55-56).

  • api_server.py — HTTP header injection: X-Hermes-Session-Id is accepted from request headers (line 534) and echoed directly into response headers (lines 675, 697) without sanitization. A session ID containing \r\n enables HTTP response splitting. Fix: reject session IDs containing control characters (\r, \n, \x00) with a 400 response.

Test plan

  • DingTalk: verify legitimate https://api.dingtalk.com/... webhook URLs are accepted
  • DingTalk: verify non-DingTalk URLs (e.g., http://169.254.169.254/) are silently rejected
  • DingTalk: verify cache eviction when exceeding 500 entries
  • API Server: verify normal session IDs are accepted
  • API Server: verify session IDs with \r\n are rejected with 400

🤖 Generated with Claude Code

…ache, reject header injection

dingtalk.py: The session_webhook URL from incoming DingTalk messages is POSTed to
without any origin validation (line 290), enabling SSRF attacks via crafted webhook
URLs (e.g. http://169.254.169.254/ to reach cloud metadata).  Add a regex check
that only accepts the official DingTalk API origin (https://api.dingtalk.com/).
Also cap _session_webhooks dict at 500 entries with FIFO eviction to prevent
unbounded memory growth from long-running gateway instances.

api_server.py: The X-Hermes-Session-Id request header is accepted and echoed back
into response headers (lines 675, 697) without sanitization.  A session ID
containing \r\n enables HTTP response splitting / header injection.  Add a check
that rejects session IDs containing control characters (\r, \n, \x00).

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #7094. Your commit was cherry-picked onto current main with your authorship preserved in git log. We also resolved a merge conflict with the new session auth gate from #6930. Thanks for the security hardening!

@teknium1 teknium1 closed this Apr 10, 2026
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