Skip to content

fix(security): add body size limit and defusedxml to prevent XML DoS before signature verification in WeCom callback#10192

Open
memosr wants to merge 1 commit into
NousResearch:mainfrom
memosr:fix/wecom-xml-parse-before-auth
Open

fix(security): add body size limit and defusedxml to prevent XML DoS before signature verification in WeCom callback#10192
memosr wants to merge 1 commit into
NousResearch:mainfrom
memosr:fix/wecom-xml-parse-before-auth

Conversation

@memosr

@memosr memosr commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

gateway/platforms/wecom_callback.py parsed untrusted XML before
verifying the WeCom signature, on an internet-facing endpoint
(DEFAULT_HOST = "0.0.0.0").

The vulnerability

# Before (vulnerable)
async def _handle_callback(self, request):
    body = await request.text()
    # ...
    decrypted = self._decrypt_request(app, body, ...)  # XML parsed here, auth later

Inside _decrypt_request:

root = ET.fromstring(body)   # ← untrusted XML parsed BEFORE signature check
encrypt = root.findtext("Encrypt", default="")
crypt.decrypt(msg_signature, ...)  # ← auth happens here, too late

Any attacker can POST malformed or malicious XML to /wecom/callback
before any credential check runs. Python's xml.etree.ElementTree
(expat-based) is not safe against malicious input — billion laughs,
deeply nested trees, and malformed content all happen pre-auth.

Fix 1 — Body size limit (64 KB)

Added a payload size check before any parsing:

_MAX_BODY = 65_536  # 64 KB is ample for any WeCom message
body_bytes = await request.read()
if len(body_bytes) > _MAX_BODY:
    return web.Response(status=413, text="payload too large")

Fix 2 — defusedxml for safe XML parsing

Replaced ET.fromstring() with defusedxml.ElementTree.fromstring()
which protects against billion laughs, quadratic blowup, and external
entity expansion:

try:
    import defusedxml.ElementTree as _safe_ET
    root = _safe_ET.fromstring(body)
except ImportError:
    root = ET.fromstring(body)  # fallback if defusedxml not installed

Type of Change

  • 🔒 Security fix (XML DoS / pre-auth parse)
  • 🔒 Security fix (oversized payload / zip bomb)

Checklist

  • Read the Contributing Guide
  • Commit messages follow Conventional Commits
  • defusedxml gracefully falls back to stdlib if not installed
  • No behavior change for legitimate WeCom callbacks

…before signature verification in WeCom callback
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery platform/wecom WeCom / WeChat Work adapter labels Apr 26, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Related to #8686 — both harden WeCom callback against XML DoS/XXE. This PR also adds body size limit (64KB). Consider closing #8686 if this supersedes it.

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

Labels

comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround platform/wecom WeCom / WeChat Work adapter type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants