From be10ecef770a4654519869c3641bbb91087c8c7b Mon Sep 17 00:00:00 2001 From: Agustin Rivera <31522568+eleqtrizit@users.noreply.github.com> Date: Thu, 2 Apr 2026 05:53:19 -0700 Subject: [PATCH] fix(compare): reuse shared secret comparison helper (#58432) * fix(compare): reuse shared secret comparison helper * fix(compare): reject empty bluebubbles auth tokens * docs: add changelog entry for shared secret comparison fix --------- Co-authored-by: Jacob Tomlinson --- CHANGELOG.md | 1 + extensions/bluebubbles/src/monitor.ts | 13 ++++--------- extensions/feishu/src/monitor.transport.ts | 12 ++---------- .../mattermost/src/mattermost/interactions.ts | 8 +++----- extensions/telegram/src/webhook.ts | 9 ++------- extensions/voice-call/src/providers/twilio.ts | 8 ++------ extensions/voice-call/src/webhook-security.ts | 19 +++---------------- extensions/zalo/src/monitor.webhook.ts | 17 ++--------------- 8 files changed, 19 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d93c187827f..30cc6265c7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Docs: https://docs.openclaw.ai - Providers/Copilot: classify native GitHub Copilot API hosts in the shared provider endpoint resolver and harden token-derived proxy endpoint parsing so Copilot base URL routing stays centralized and fails closed on malformed hints. Thanks @vincentkoc. - Gateway: prune empty `node-pending-work` state entries after explicit acknowledgments and natural expiry so the per-node state map no longer grows indefinitely. (#58179) Thanks @gavyngong. - Browser/CDP: normalize trailing-dot localhost absolute-form hosts before loopback checks so remote CDP websocket URLs like `ws://localhost.:...` rewrite back to the configured remote host. (#59236) Thanks @mappel-nv. +- Webhooks/secret comparison: replace ad-hoc timing-safe secret comparisons across BlueBubbles, Feishu, Mattermost, Telegram, Twilio, and Zalo webhook handlers with the shared `safeEqualSecret` helper and reject empty auth tokens in BlueBubbles. Thanks @eleqtrizit. ## 2026.4.1-beta.1 diff --git a/extensions/bluebubbles/src/monitor.ts b/extensions/bluebubbles/src/monitor.ts index f4688f16873..6eac5af06c2 100644 --- a/extensions/bluebubbles/src/monitor.ts +++ b/extensions/bluebubbles/src/monitor.ts @@ -1,5 +1,5 @@ -import { timingSafeEqual } from "node:crypto"; import type { IncomingMessage, ServerResponse } from "node:http"; +import { safeEqualSecret } from "openclaw/plugin-sdk/browser-support"; import { createBlueBubblesDebounceRegistry } from "./monitor-debounce.js"; import { normalizeWebhookMessage, normalizeWebhookReaction } from "./monitor-normalize.js"; import { logVerbose, processMessage, processReaction } from "./monitor-processing.js"; @@ -116,18 +116,13 @@ function normalizeAuthToken(raw: string): string { return value; } -function safeEqualSecret(aRaw: string, bRaw: string): boolean { +function safeEqualAuthToken(aRaw: string, bRaw: string): boolean { const a = normalizeAuthToken(aRaw); const b = normalizeAuthToken(bRaw); if (!a || !b) { return false; } - const bufA = Buffer.from(a, "utf8"); - const bufB = Buffer.from(b, "utf8"); - if (bufA.length !== bufB.length) { - return false; - } - return timingSafeEqual(bufA, bufB); + return safeEqualSecret(a, b); } function collectTrustedProxies(targets: readonly WebhookTarget[]): string[] { @@ -198,7 +193,7 @@ export async function handleBlueBubblesWebhookRequest( res, isMatch: (target) => { const token = target.account.config.password?.trim() ?? ""; - return safeEqualSecret(guid, token); + return safeEqualAuthToken(guid, token); }, }); if (!target) { diff --git a/extensions/feishu/src/monitor.transport.ts b/extensions/feishu/src/monitor.transport.ts index 0ffa1b80cb7..22b3d221887 100644 --- a/extensions/feishu/src/monitor.transport.ts +++ b/extensions/feishu/src/monitor.transport.ts @@ -1,6 +1,7 @@ import * as http from "http"; import crypto from "node:crypto"; import * as Lark from "@larksuiteoapi/node-sdk"; +import { safeEqualSecret } from "openclaw/plugin-sdk/browser-support"; import { applyBasicWebhookRequestGuards, isRequestBodyLimitError, @@ -34,15 +35,6 @@ function isFeishuWebhookPayload(value: unknown): value is Record, @@ -83,7 +75,7 @@ function isFeishuWebhookSignatureValid(params: { .createHash("sha256") .update(timestamp + nonce + encryptKey + params.rawBody) .digest("hex"); - return timingSafeEqualString(computedSignature, signature); + return safeEqualSecret(computedSignature, signature); } function respondText(res: http.ServerResponse, statusCode: number, body: string): void { diff --git a/extensions/mattermost/src/mattermost/interactions.ts b/extensions/mattermost/src/mattermost/interactions.ts index 90550fcbba3..1ede1174be3 100644 --- a/extensions/mattermost/src/mattermost/interactions.ts +++ b/extensions/mattermost/src/mattermost/interactions.ts @@ -1,5 +1,6 @@ -import { createHmac, timingSafeEqual } from "node:crypto"; +import { createHmac } from "node:crypto"; import type { IncomingMessage, ServerResponse } from "node:http"; +import { safeEqualSecret } from "openclaw/plugin-sdk/browser-support"; import { getMattermostRuntime } from "../runtime.js"; import { updateMattermostPost, type MattermostClient, type MattermostPost } from "./client.js"; import { isTrustedProxyAddress, resolveClientIp, type OpenClawConfig } from "./runtime-api.js"; @@ -223,10 +224,7 @@ export function verifyInteractionToken( accountId?: string, ): boolean { const expected = generateInteractionToken(context, accountId); - if (expected.length !== token.length) { - return false; - } - return timingSafeEqual(Buffer.from(expected), Buffer.from(token)); + return safeEqualSecret(expected, token); } // ── Button builder helpers ───────────────────────────────────────────── diff --git a/extensions/telegram/src/webhook.ts b/extensions/telegram/src/webhook.ts index a6e062fbe8c..034821714eb 100644 --- a/extensions/telegram/src/webhook.ts +++ b/extensions/telegram/src/webhook.ts @@ -1,8 +1,8 @@ -import { timingSafeEqual } from "node:crypto"; import { createServer } from "node:http"; import type { IncomingMessage } from "node:http"; import net from "node:net"; import * as grammy from "grammy"; +import { safeEqualSecret } from "openclaw/plugin-sdk/browser-support"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import { isDiagnosticsEnabled } from "openclaw/plugin-sdk/diagnostic-runtime"; import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env"; @@ -102,12 +102,7 @@ function hasValidTelegramWebhookSecret( secretHeader: string | undefined, expectedSecret: string, ): boolean { - if (typeof secretHeader !== "string") { - return false; - } - const actual = Buffer.from(secretHeader, "utf-8"); - const expected = Buffer.from(expectedSecret, "utf-8"); - return actual.length === expected.length && timingSafeEqual(actual, expected); + return safeEqualSecret(secretHeader, expectedSecret); } function parseIpLiteral(value: string | undefined): string | undefined { diff --git a/extensions/voice-call/src/providers/twilio.ts b/extensions/voice-call/src/providers/twilio.ts index 231ce3150d0..b7e9064ff07 100644 --- a/extensions/voice-call/src/providers/twilio.ts +++ b/extensions/voice-call/src/providers/twilio.ts @@ -1,4 +1,5 @@ import crypto from "node:crypto"; +import { safeEqualSecret } from "openclaw/plugin-sdk/browser-support"; import type { TwilioConfig, WebhookSecurityConfig } from "../config.js"; import { getHeader } from "../http-headers.js"; import type { MediaStreamHandler } from "../media-stream.js"; @@ -205,12 +206,7 @@ export class TwilioProvider implements VoiceCallProvider { if (!expected || !token) { return false; } - if (expected.length !== token.length) { - const dummy = Buffer.from(expected); - crypto.timingSafeEqual(dummy, dummy); - return false; - } - return crypto.timingSafeEqual(Buffer.from(expected), Buffer.from(token)); + return safeEqualSecret(expected, token); } /** diff --git a/extensions/voice-call/src/webhook-security.ts b/extensions/voice-call/src/webhook-security.ts index a6ff0f6a674..02b92d6a975 100644 --- a/extensions/voice-call/src/webhook-security.ts +++ b/extensions/voice-call/src/webhook-security.ts @@ -1,4 +1,5 @@ import crypto from "node:crypto"; +import { safeEqualSecret } from "openclaw/plugin-sdk/browser-support"; import { getHeader } from "./http-headers.js"; import type { WebhookContext } from "./types.js"; @@ -120,16 +121,7 @@ function buildCanonicalTwilioParamString(params: URLSearchParams): string { * Timing-safe string comparison to prevent timing attacks. */ function timingSafeEqual(a: string, b: string): boolean { - if (a.length !== b.length) { - // Still do comparison to maintain constant time - const dummy = Buffer.from(a); - crypto.timingSafeEqual(dummy, dummy); - return false; - } - - const bufA = Buffer.from(a); - const bufB = Buffer.from(b); - return crypto.timingSafeEqual(bufA, bufB); + return safeEqualSecret(a, b); } /** @@ -745,12 +737,7 @@ function createPlivoV3ReplayKey(params: { } function timingSafeEqualString(a: string, b: string): boolean { - if (a.length !== b.length) { - const dummy = Buffer.from(a); - crypto.timingSafeEqual(dummy, dummy); - return false; - } - return crypto.timingSafeEqual(Buffer.from(a), Buffer.from(b)); + return safeEqualSecret(a, b); } function validatePlivoV2Signature(params: { diff --git a/extensions/zalo/src/monitor.webhook.ts b/extensions/zalo/src/monitor.webhook.ts index 647db1da395..155c547a4e0 100644 --- a/extensions/zalo/src/monitor.webhook.ts +++ b/extensions/zalo/src/monitor.webhook.ts @@ -1,5 +1,5 @@ -import { timingSafeEqual } from "node:crypto"; import type { IncomingMessage, ServerResponse } from "node:http"; +import { safeEqualSecret } from "openclaw/plugin-sdk/browser-support"; import type { ResolvedZaloAccount } from "./accounts.js"; import type { ZaloFetch, ZaloUpdate } from "./api.js"; import type { ZaloRuntimeEnv } from "./monitor.js"; @@ -72,20 +72,7 @@ export function getZaloWebhookStatusCounterSizeForTest(): number { } function timingSafeEquals(left: string, right: string): boolean { - const leftBuffer = Buffer.from(left); - const rightBuffer = Buffer.from(right); - - if (leftBuffer.length !== rightBuffer.length) { - const length = Math.max(1, leftBuffer.length, rightBuffer.length); - const paddedLeft = Buffer.alloc(length); - const paddedRight = Buffer.alloc(length); - leftBuffer.copy(paddedLeft); - rightBuffer.copy(paddedRight); - timingSafeEqual(paddedLeft, paddedRight); - return false; - } - - return timingSafeEqual(leftBuffer, rightBuffer); + return safeEqualSecret(left, right); } function isReplayEvent(target: ZaloWebhookTarget, update: ZaloUpdate, nowMs: number): boolean {