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

This commit is contained in:
gavyngong
2026-04-01 10:16:48 +08:00
committed by Mason Huang
parent 6bc3458222
commit 60b4d43e33
2 changed files with 111 additions and 9 deletions

View File

@@ -131,6 +131,99 @@ describe("nextcloud talk core", () => {
).toBeNull();
});
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("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("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;
}
/**