Skip to content

fix(security): use hmac.compare_digest for BlueBubbles webhook token to prevent timing attacks - #9219

Open
memosr wants to merge 1 commit into
NousResearch:mainfrom
memosr:fix/bluebubbles-timing-attack
Open

fix(security): use hmac.compare_digest for BlueBubbles webhook token to prevent timing attacks#9219
memosr wants to merge 1 commit into
NousResearch:mainfrom
memosr:fix/bluebubbles-timing-attack

Conversation

@memosr

@memosr memosr commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

gateway/platforms/bluebubbles.py validated the webhook token using
Python's != operator:

# Before (vulnerable)
if token != self.password:
    return web.json_response({"error": "unauthorized"}, status=401)

String inequality with != short-circuits at the first mismatched
character. An attacker can measure response times to determine how
many leading characters of their guess match the real token,
eventually recovering it character by character (timing oracle attack).

Fix

Replaced with hmac.compare_digest() which always takes the same
time regardless of where the strings differ:

# After (safe)
import hmac
if not hmac.compare_digest(token or "", self.password or ""):
    return web.json_response({"error": "unauthorized"}, status=401)

The or "" guards handle None values safely since
hmac.compare_digest() requires string arguments.

Type of Change

  • 🔒 Security fix (timing attack)

Checklist

@alt-glitch alt-glitch added type/security Security vulnerability or hardening P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels Apr 27, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing a real timing-sensitive webhook-auth path. Current main still uses token != self.password at gateway/platforms/bluebubbles.py:883, so the security premise is valid.

Problems

  • The added hmac.compare_digest(token or "", self.password or "") compares raw str values. Python documents string operands as ASCII-only; the adapter accepts BLUEBUBBLES_PASSWORD directly at gateway/platforms/bluebubbles.py:128, without ASCII validation. A non-ASCII password could therefore raise rather than return the existing 401 behavior.
  • No regression tests accompany this authentication change. tests/gateway/test_bluebubbles.py currently lacks wrong-password, missing-credential, and comparator-invocation coverage.

Suggested changes

  • Compare UTF-8 encoded operands so all configured string passwords are handled safely.
  • Add focused auth tests for wrong/missing credentials and constant-time comparator use. The timeline's cross-referenced open duplicate #44025 already illustrates both pieces.

Automated hermes-sweeper review.

@@ -770,7 +771,7 @@ async def _handle_webhook(self, request):
or request.headers.get("x-guid")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

compare_digest only accepts ASCII str operands. This adapter accepts arbitrary BLUEBUBBLES_PASSWORD strings, so encode both operands to UTF-8 bytes here; otherwise a non-ASCII password can raise instead of returning the existing 401 response.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@memosr
memosr force-pushed the fix/bluebubbles-timing-attack branch from bf7c7ec to 014bbde Compare July 12, 2026 19:26
@memosr

memosr commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both sweeper findings:

  1. Non-ASCII password crash: hmac.compare_digest raises TypeError on non-ASCII str inputs, which would have turned a wrong password into a 500. The comparison now encodes both sides to UTF-8 bytes: hmac.compare_digest((token or "").encode("utf-8"), (self.password or "").encode("utf-8")) — preserving the existing 401 behavior for any password value.

  2. Missing regression coverage: added a TestBlueBubblesWebhookAuth suite to tests/gateway/test_bluebubbles.py covering: wrong token → 401, missing/empty token → 401, non-ASCII password ("şifre123") with correct token accepted and wrong token → 401 (no exception), and a spy test asserting the comparison actually goes through hmac.compare_digest with the encoded bytes.

All 67 tests in the file pass. Rebased onto current main; still a single commit.

@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

The webhook authentication fix is correctly applied at the only BlueBubbles webhook entrypoint. It replaces the ordinary token inequality with hmac.compare_digest over UTF-8 bytes, preserves the early 401 rejection, and keeps valid and invalid non-ASCII credentials from raising. Focused regression tests cover missing, empty, wrong, correct, and non-ASCII credential cases; no residual authentication bypass was found.

Review note: I used a run-owned local patch replay against current GitHub main; this does not mean the submitted branch itself merges cleanly.

Security evidence:

  • trust boundary: An unauthenticated BlueBubbles HTTP POST reaches _handle_webhook. The request supplies a token through the supported query or header fields, while self.password is the configured secret. The handler must authenticate before reading or parsing the request body or scheduling message work.
  • source/sink/invariant: The handler extracts supported token sources, encodes both values as UTF-8 bytes, and compares them with hmac.compare_digest; mismatches return 401 before body access, and only a match reaches payload parsing and message dispatch. connect also refuses to expose the listener without a non-empty password.
  • current-main reproduction: Current main used ordinary token != self.password at the webhook gate, which is prefix-dependent and exposes a timing oracle. The patch replaces that predicate with hmac.compare_digest on UTF-8 bytes at the same gate, preserving the rejection response and control-flow boundary.
  • PR-head or patch-replay validation: The patch changes the webhook comparison and adds focused authentication tests; the focused suite passes and the authentication control flow remains intact.
  • positive/negative cases: Regression cases cover 401 for wrong, missing, and empty tokens plus an invalid token with a non-ASCII configured password; 200 for correct ASCII and non-ASCII credentials; and verify that hmac.compare_digest receives byte arguments.
  • residual bypass search: Inspected every token source and return path in _handle_webhook, listener setup, password initialization, and sibling token-comparison patterns. No alternate BlueBubbles webhook handler or pre-auth body/dispatch path remains, and no newly introduced bypass was found.
  • reviewer validation: Independent source review and focused tests found no additional BlueBubbles authentication issue.

Not checked:

  • Live BlueBubbles server or network timing measurement

Signed: GPT-5.6-luna-max in Codex

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 P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants