fix(nextcloud-talk): replace manual XOR with crypto.timingSafeEqual and fix length leak (#58097)

Merged via squash.

Prepared head SHA: 3cb82bce40
Co-authored-by: gavyngong <267269824+gavyngong@users.noreply.github.com>
Co-authored-by: hxy91819 <8814856+hxy91819@users.noreply.github.com>
Reviewed-by: @hxy91819
This commit is contained in:
gavyngong
2026-05-01 10:26:54 +08:00
committed by GitHub
parent 206b5f78a2
commit fd0ca5987b
3 changed files with 113 additions and 10 deletions

View File

@@ -23,6 +23,7 @@ Docs: https://docs.openclaw.ai
- Gateway/systemd: exit with sysexits 78 for supervised lock and `EADDRINUSE` conflicts so `RestartPreventExitStatus=78` stops `Restart=always` restart loops instead of repeatedly reloading plugins against an occupied port. Fixes #75115. Thanks @yhyatt.
- Agents/runtime: skip blank visible user prompts at the embedded-runner boundary before provider submission while still allowing internal runtime-only turns and media-only prompts, so Telegram/group sessions no longer leak raw empty-input provider errors when replay history exists. Fixes #74137. Thanks @yelog, @Gracker, and @nhaener.
- Agents/Codex: isolate local Codex app-server `CODEX_HOME` and `HOME` per agent and add a deliberate Codex migration path with selectable skill copies, so personal Codex CLI skills, plugins, config, and hooks no longer leak into OpenClaw agents unless the operator migrates them into the workspace. Thanks @pashpashpash.
- Security/Nextcloud Talk: make webhook signature validation use the padded timing-safe compare path even when the supplied signature length is wrong, keep normalized header lookup behavior, and extend regression coverage for tampered bodies, wrong secrets, array-backed headers, and truncated signatures. Carries forward earlier contributor work from #50516 by teddytennant. (#58097) Thanks @gavyngong.
- Plugins/runtime-deps: replace stale symlinked mirror target roots before writing runtime-mirror temp files and skip rewriting already materialized hardlinks, so cross-version container upgrades no longer crash-loop on read-only image-layer paths while warm mirrors do less churn. Fixes #75108; refs #75069. Thanks @coletebou and @xiaohuaxi.
- Auto-reply/group chats: fall back to automatic source delivery when a channel precomputes message-tool-only replies but the `message` tool is unavailable, so Discord/Slack-style group turns do not silently complete without a visible reply. Fixes #74868. Thanks @kagura-agent.
- Browser/gateway: share one browser control runtime across the HTTP control server and `browser.request`, and refresh browser profile config from the source snapshot, so CLI status/start honors configured `browser.executablePath`, `headless`, and `noSandbox` instead of falling back to stale auto-detection. Fixes #75087; repairs #73617. Thanks @civiltox and @martingarramon.

View File

@@ -1,7 +1,7 @@
import { mkdtemp, rm } from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import {
looksLikeNextcloudTalkTargetId,
normalizeNextcloudTalkMessagingTarget,
@@ -131,6 +131,99 @@ describe("nextcloud talk core", () => {
).toBeNull();
});
it("rejects tampered bodies, wrong secrets, and tampered signatures", () => {
const body = JSON.stringify({ hello: "world" });
const generated = generateNextcloudTalkSignature({
body,
secret: "secret-123",
});
expect(
verifyNextcloudTalkSignature({
signature: generated.signature,
random: generated.random,
body: JSON.stringify({ hello: "tampered" }),
secret: "secret-123",
}),
).toBe(false);
expect(
verifyNextcloudTalkSignature({
signature: generated.signature,
random: generated.random,
body,
secret: "wrong-secret",
}),
).toBe(false);
expect(
verifyNextcloudTalkSignature({
signature: "a".repeat(generated.signature.length),
random: generated.random,
body,
secret: "secret-123",
}),
).toBe(false);
});
it("takes the first value from array-backed headers", () => {
expect(
extractNextcloudTalkHeaders({
"x-nextcloud-talk-signature": ["sig1", "sig2"],
"x-nextcloud-talk-random": ["rand1", "rand2"],
"x-nextcloud-talk-backend": ["backend1", "backend2"],
}),
).toEqual({
signature: "sig1",
random: "rand1",
backend: "backend1",
});
});
it("still runs timingSafeEqual when the supplied signature length mismatches", async () => {
const timingSafeEqualMock = vi.fn();
vi.resetModules();
vi.doMock("node:crypto", async (importOriginal) => {
const actual = await importOriginal<typeof import("node:crypto")>();
return {
...actual,
timingSafeEqual: vi.fn((left: NodeJS.ArrayBufferView, right: NodeJS.ArrayBufferView) => {
timingSafeEqualMock(left, right);
return actual.timingSafeEqual(left, right);
}),
};
});
const { generateNextcloudTalkSignature, verifyNextcloudTalkSignature } =
await import("./signature.js");
const body = JSON.stringify({ hello: "world" });
const generated = generateNextcloudTalkSignature({
body,
secret: "secret-123",
});
const shortSignature = generated.signature.slice(0, 12);
expect(
verifyNextcloudTalkSignature({
signature: shortSignature,
random: generated.random,
body,
secret: "secret-123",
}),
).toBe(false);
expect(timingSafeEqualMock).toHaveBeenCalledOnce();
const [leftBuffer, rightBuffer] = timingSafeEqualMock.mock.calls[0] ?? [];
expect(Buffer.isBuffer(leftBuffer)).toBe(true);
expect(Buffer.isBuffer(rightBuffer)).toBe(true);
if (!Buffer.isBuffer(leftBuffer) || !Buffer.isBuffer(rightBuffer)) {
throw new TypeError("Expected timingSafeEqual to receive Buffer arguments");
}
expect(leftBuffer).toHaveLength(rightBuffer.length);
vi.doUnmock("node:crypto");
vi.resetModules();
});
it("persists replay decisions across guard instances and scopes account namespaces", async () => {
const stateDir = await makeTempDir();

View File

@@ -1,4 +1,4 @@
import { createHmac, randomBytes } from "node:crypto";
import { createHmac, randomBytes, timingSafeEqual } from "node:crypto";
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/text-runtime";
import type { NextcloudTalkWebhookHeaders } from "./types.js";
@@ -25,14 +25,23 @@ export function verifyNextcloudTalkSignature(params: {
.update(random + body)
.digest("hex");
if (signature.length !== expected.length) {
return false;
}
let result = 0;
for (let i = 0; i < signature.length; i++) {
result |= signature.charCodeAt(i) ^ expected.charCodeAt(i);
}
return result === 0;
const expectedBuf = Buffer.from(expected, "utf8");
const signatureBuf = Buffer.from(signature, "utf8");
// Pad to equal length before constant-time comparison to prevent
// leaking length information via early-return timing.
// Note: digest("hex") always produces lowercase ASCII (64 bytes for SHA-256),
// so expectedBuf is always 64 bytes — no variable-length concern on the expected side.
const maxLen = Math.max(expectedBuf.length, signatureBuf.length);
const paddedExpected = Buffer.alloc(maxLen);
const paddedSignature = Buffer.alloc(maxLen);
expectedBuf.copy(paddedExpected);
signatureBuf.copy(paddedSignature);
// Use crypto.timingSafeEqual instead of manual XOR loop to avoid
// potential JIT-optimisation timing leaks in the JavaScript engine.
const timingResult = timingSafeEqual(paddedExpected, paddedSignature);
return expectedBuf.length === signatureBuf.length && timingResult;
}
/**