fix(security): widen non-ASCII compare_digest crash fix to all sibling sites
Same bug class as the salvaged #65305/#65307: hmac.compare_digest (and secrets.compare_digest) raise TypeError when given a str containing non-ASCII characters, and these call sites feed it raw request input. Compare as UTF-8 bytes everywhere: - gateway/platforms/msgraph_webhook.py: clientState from request body - gateway/platforms/whatsapp_cloud.py: hub.verify_token query param + X-Hub-Signature-256 header (comment claimed 'works on str' — it doesn't for non-ASCII) - plugins/platforms/feishu: verification token + x-lark-signature - plugins/platforms/raft: bridge token header - plugins/platforms/line: X-Line-Signature - plugins/platforms/sms: X-Twilio-Signature - tools/code_execution_tool.py: sandbox RPC token (both loops) Regression tests for the two gateway-core sites (msgraph, whatsapp).fix/verification-admin-route-recovery
parent
4ccb232af9
commit
a6d9d1d2cf
|
|
@ -358,7 +358,9 @@ class MSGraphWebhookAdapter(BasePlatformAdapter):
|
||||||
provided = self._string_or_none(notification.get("clientState"))
|
provided = self._string_or_none(notification.get("clientState"))
|
||||||
if provided is None:
|
if provided is None:
|
||||||
return False
|
return False
|
||||||
return hmac.compare_digest(provided, expected)
|
# Compare as bytes: ``compare_digest`` raises TypeError on a str with
|
||||||
|
# non-ASCII characters, and clientState comes from the request body.
|
||||||
|
return hmac.compare_digest(provided.encode(), expected.encode())
|
||||||
|
|
||||||
def _has_seen_receipt(self, receipt_key: str) -> bool:
|
def _has_seen_receipt(self, receipt_key: str) -> bool:
|
||||||
return receipt_key in self._seen_receipts
|
return receipt_key in self._seen_receipts
|
||||||
|
|
|
||||||
|
|
@ -1413,10 +1413,11 @@ class WhatsAppCloudAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
|
||||||
return web.Response(status=400, text="bad mode")
|
return web.Response(status=400, text="bad mode")
|
||||||
|
|
||||||
# Constant-time compare to avoid token-length / token-content leaks
|
# Constant-time compare to avoid token-length / token-content leaks
|
||||||
# via timing. ``hmac.compare_digest`` works on str.
|
# via timing. Compare as bytes: ``compare_digest`` raises TypeError on
|
||||||
|
# a str with non-ASCII characters, and the token is a raw query param.
|
||||||
import hmac as _hmac
|
import hmac as _hmac
|
||||||
|
|
||||||
if not _hmac.compare_digest(token, self._verify_token):
|
if not _hmac.compare_digest(token.encode(), self._verify_token.encode()):
|
||||||
return web.Response(status=403, text="verify_token mismatch")
|
return web.Response(status=403, text="verify_token mismatch")
|
||||||
if not challenge:
|
if not challenge:
|
||||||
return web.Response(status=400, text="missing challenge")
|
return web.Response(status=400, text="missing challenge")
|
||||||
|
|
@ -1509,7 +1510,11 @@ class WhatsAppCloudAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
|
||||||
raw_body,
|
raw_body,
|
||||||
hashlib.sha256,
|
hashlib.sha256,
|
||||||
).hexdigest()
|
).hexdigest()
|
||||||
return hmac.compare_digest(computed.lower(), expected_hex.lower())
|
# Compare as bytes: compare_digest raises TypeError on a str with
|
||||||
|
# non-ASCII characters, and the signature is a raw request header.
|
||||||
|
return hmac.compare_digest(
|
||||||
|
computed.lower().encode(), expected_hex.lower().encode()
|
||||||
|
)
|
||||||
|
|
||||||
# ------------------------------------------------------------------ dispatch
|
# ------------------------------------------------------------------ dispatch
|
||||||
def _dedup_wamid(self, wamid: str) -> bool:
|
def _dedup_wamid(self, wamid: str) -> bool:
|
||||||
|
|
|
||||||
|
|
@ -3512,7 +3512,11 @@ class FeishuAdapter(BasePlatformAdapter):
|
||||||
if self._verification_token:
|
if self._verification_token:
|
||||||
header = payload.get("header") or {}
|
header = payload.get("header") or {}
|
||||||
incoming_token = str(header.get("token") or payload.get("token") or "")
|
incoming_token = str(header.get("token") or payload.get("token") or "")
|
||||||
if not incoming_token or not hmac.compare_digest(incoming_token, self._verification_token):
|
# Compare as bytes: compare_digest raises TypeError on a str with
|
||||||
|
# non-ASCII characters, and the token comes from the request body.
|
||||||
|
if not incoming_token or not hmac.compare_digest(
|
||||||
|
incoming_token.encode(), self._verification_token.encode()
|
||||||
|
):
|
||||||
logger.warning("[Feishu] Webhook rejected: invalid verification token from %s", remote_ip)
|
logger.warning("[Feishu] Webhook rejected: invalid verification token from %s", remote_ip)
|
||||||
self._record_webhook_anomaly(remote_ip, "401-token")
|
self._record_webhook_anomaly(remote_ip, "401-token")
|
||||||
return web.Response(status=401, text="Invalid verification token")
|
return web.Response(status=401, text="Invalid verification token")
|
||||||
|
|
@ -3575,7 +3579,9 @@ class FeishuAdapter(BasePlatformAdapter):
|
||||||
body_str = body_bytes.decode("utf-8", errors="replace")
|
body_str = body_bytes.decode("utf-8", errors="replace")
|
||||||
content = f"{timestamp}{nonce}{self._encrypt_key}{body_str}"
|
content = f"{timestamp}{nonce}{self._encrypt_key}{body_str}"
|
||||||
computed = hashlib.sha256(content.encode("utf-8")).hexdigest()
|
computed = hashlib.sha256(content.encode("utf-8")).hexdigest()
|
||||||
return hmac.compare_digest(computed, signature)
|
# Compare as bytes: compare_digest raises TypeError on a str with
|
||||||
|
# non-ASCII characters, and the signature is a raw request header.
|
||||||
|
return hmac.compare_digest(computed.encode(), signature.encode())
|
||||||
except Exception:
|
except Exception:
|
||||||
logger.debug("[Feishu] Signature verification raised an exception", exc_info=True)
|
logger.debug("[Feishu] Signature verification raised an exception", exc_info=True)
|
||||||
return False
|
return False
|
||||||
|
|
|
||||||
|
|
@ -273,7 +273,9 @@ def verify_line_signature(body: bytes, signature: str, channel_secret: str) -> b
|
||||||
expected = base64.b64encode(digest).decode("utf-8")
|
expected = base64.b64encode(digest).decode("utf-8")
|
||||||
except Exception:
|
except Exception:
|
||||||
return False
|
return False
|
||||||
return hmac.compare_digest(expected, signature)
|
# Compare as bytes: compare_digest raises TypeError on a str with
|
||||||
|
# non-ASCII characters, and the signature is a raw request header.
|
||||||
|
return hmac.compare_digest(expected.encode(), signature.encode())
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
|
|
@ -692,7 +692,9 @@ class RaftAdapter(BasePlatformAdapter):
|
||||||
def _validate_bridge_token(self, token: str) -> bool:
|
def _validate_bridge_token(self, token: str) -> bool:
|
||||||
if not self._bridge_token or not token:
|
if not self._bridge_token or not token:
|
||||||
return False
|
return False
|
||||||
return hmac.compare_digest(token, self._bridge_token)
|
# Compare as bytes: compare_digest raises TypeError on a str with
|
||||||
|
# non-ASCII characters, and the token is a raw request header.
|
||||||
|
return hmac.compare_digest(token.encode(), self._bridge_token.encode())
|
||||||
|
|
||||||
async def _accept_wake(self, payload: Dict[str, Any]) -> bool:
|
async def _accept_wake(self, payload: Dict[str, Any]) -> bool:
|
||||||
if not self._message_handler:
|
if not self._message_handler:
|
||||||
|
|
|
||||||
|
|
@ -257,7 +257,9 @@ class SmsAdapter(BasePlatformAdapter):
|
||||||
hashlib.sha1,
|
hashlib.sha1,
|
||||||
)
|
)
|
||||||
computed = base64.b64encode(mac.digest()).decode("utf-8")
|
computed = base64.b64encode(mac.digest()).decode("utf-8")
|
||||||
return hmac.compare_digest(computed, signature)
|
# Compare as bytes: compare_digest raises TypeError on a str with
|
||||||
|
# non-ASCII characters, and the signature is a raw request header.
|
||||||
|
return hmac.compare_digest(computed.encode(), signature.encode())
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _port_variant_url(url: str) -> str | None:
|
def _port_variant_url(url: str) -> str | None:
|
||||||
|
|
|
||||||
|
|
@ -316,8 +316,30 @@ class TestMSGraphNotifications:
|
||||||
|
|
||||||
assert calls, "hmac.compare_digest was never called; clientState check is not timing-safe"
|
assert calls, "hmac.compare_digest was never called; clientState check is not timing-safe"
|
||||||
provided, expected = calls[0]
|
provided, expected = calls[0]
|
||||||
assert provided == "expected-client-state"
|
assert provided == b"expected-client-state"
|
||||||
assert expected == "expected-client-state"
|
assert expected == b"expected-client-state"
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_non_ascii_client_state_rejected_without_raising(self):
|
||||||
|
"""A non-ASCII clientState (attacker-controlled request body) must be
|
||||||
|
rejected with 403, not crash the handler: hmac.compare_digest raises
|
||||||
|
TypeError on a str containing non-ASCII characters."""
|
||||||
|
adapter = _make_adapter()
|
||||||
|
payload = {
|
||||||
|
"value": [
|
||||||
|
{
|
||||||
|
"id": "notif-nonascii",
|
||||||
|
"subscriptionId": "sub-1",
|
||||||
|
"changeType": "updated",
|
||||||
|
"resource": "communications/onlineMeetings/meeting-x",
|
||||||
|
"clientState": "ské-not-the-secret",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
|
response = await adapter._handle_notification(
|
||||||
|
_FakeRequest(json_payload=payload)
|
||||||
|
)
|
||||||
|
assert response.status == 403
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
async def test_duplicate_notification_deduped(self):
|
async def test_duplicate_notification_deduped(self):
|
||||||
|
|
|
||||||
|
|
@ -353,6 +353,22 @@ class TestWebhookVerify:
|
||||||
|
|
||||||
assert response.status == 403
|
assert response.status == 403
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_verify_rejects_non_ascii_token_without_raising(self):
|
||||||
|
"""A non-ASCII verify_token (raw query param) must be rejected with
|
||||||
|
403, not crash the handler: hmac.compare_digest raises TypeError on a
|
||||||
|
str containing non-ASCII characters."""
|
||||||
|
adapter = _make_adapter(verify_token="shared-secret-123")
|
||||||
|
request = _verify_request({
|
||||||
|
"hub.mode": "subscribe",
|
||||||
|
"hub.verify_token": "ské-not-the-secret",
|
||||||
|
"hub.challenge": "abc-12345",
|
||||||
|
})
|
||||||
|
|
||||||
|
response = await adapter._handle_verify(request)
|
||||||
|
|
||||||
|
assert response.status == 403
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_verify_rejects_wrong_mode(self):
|
async def test_verify_rejects_wrong_mode(self):
|
||||||
adapter = _make_adapter(verify_token="shared-secret-123")
|
adapter = _make_adapter(verify_token="shared-secret-123")
|
||||||
|
|
|
||||||
|
|
@ -596,7 +596,10 @@ def _rpc_server_loop(
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if not rpc_token or not secrets.compare_digest(
|
if not rpc_token or not secrets.compare_digest(
|
||||||
str(request.get("token") or ""), rpc_token
|
# Compare as bytes: compare_digest raises TypeError on a
|
||||||
|
# str with non-ASCII characters, and the token comes from
|
||||||
|
# sandbox-script-supplied JSON.
|
||||||
|
str(request.get("token") or "").encode(), rpc_token.encode()
|
||||||
):
|
):
|
||||||
resp = json.dumps({"error": "Unauthorized RPC request"})
|
resp = json.dumps({"error": "Unauthorized RPC request"})
|
||||||
conn.sendall((resp + "\n").encode())
|
conn.sendall((resp + "\n").encode())
|
||||||
|
|
@ -881,7 +884,10 @@ def _rpc_poll_loop(
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if not rpc_token or not secrets.compare_digest(
|
if not rpc_token or not secrets.compare_digest(
|
||||||
str(request.get("token") or ""), rpc_token
|
# Compare as bytes: compare_digest raises TypeError on a
|
||||||
|
# str with non-ASCII characters, and the token comes from
|
||||||
|
# sandbox-script-supplied JSON.
|
||||||
|
str(request.get("token") or "").encode(), rpc_token.encode()
|
||||||
):
|
):
|
||||||
logger.debug("Unauthorized RPC request in %s", req_file)
|
logger.debug("Unauthorized RPC request in %s", req_file)
|
||||||
env.execute(f"rm -f {quoted_req_file}", cwd="/", timeout=5)
|
env.execute(f"rm -f {quoted_req_file}", cwd="/", timeout=5)
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue