diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 476f4f0..7de7f7a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -12,3 +12,4 @@ export type { ISessionStore } from "./domain/session-store"; export type { ITransport } from "./domain/transport"; export { DEFAULT_SESSION_TTL, SessionStore } from "./session-store"; export { WebSocketTransport, type WebSocketTransportOptions } from "./transport/websocket/index"; +export { timingSafeEqual } from "./utils/timing-safe-equal"; diff --git a/packages/core/src/utils/timing-safe-equal.test.ts b/packages/core/src/utils/timing-safe-equal.test.ts new file mode 100644 index 0000000..f193135 --- /dev/null +++ b/packages/core/src/utils/timing-safe-equal.test.ts @@ -0,0 +1,30 @@ +import * as t from "vitest"; +import { timingSafeEqual } from "./timing-safe-equal"; + +t.describe("timingSafeEqual", () => { + t.test("returns true for identical strings", () => { + t.expect(timingSafeEqual("123456", "123456")).toBe(true); + }); + + t.test("returns true for empty strings", () => { + t.expect(timingSafeEqual("", "")).toBe(true); + }); + + t.test("returns false for different strings of same length", () => { + t.expect(timingSafeEqual("123456", "654321")).toBe(false); + }); + + t.test("returns false when only last character differs", () => { + t.expect(timingSafeEqual("123456", "123457")).toBe(false); + }); + + t.test("returns false for different lengths", () => { + t.expect(timingSafeEqual("12345", "123456")).toBe(false); + t.expect(timingSafeEqual("123456", "12345")).toBe(false); + }); + + t.test("returns false when one string is empty", () => { + t.expect(timingSafeEqual("", "123456")).toBe(false); + t.expect(timingSafeEqual("123456", "")).toBe(false); + }); +}); diff --git a/packages/core/src/utils/timing-safe-equal.ts b/packages/core/src/utils/timing-safe-equal.ts new file mode 100644 index 0000000..c66fbdd --- /dev/null +++ b/packages/core/src/utils/timing-safe-equal.ts @@ -0,0 +1,17 @@ +/** + * Constant-time string comparison to prevent timing side-channel attacks. + * + * Always compares every character regardless of where a mismatch occurs, + * so the execution time does not leak information about which characters + * matched. Returns false immediately only when the lengths differ (length + * is not considered secret for OTP comparison). + */ +export function timingSafeEqual(a: string, b: string): boolean { + if (a.length !== b.length) return false; + + let result = 0; + for (let i = 0; i < a.length; i++) { + result |= a.charCodeAt(i) ^ b.charCodeAt(i); + } + return result === 0; +} diff --git a/packages/dapp-client/CHANGELOG.md b/packages/dapp-client/CHANGELOG.md index 06b7386..f24a42c 100644 --- a/packages/dapp-client/CHANGELOG.md +++ b/packages/dapp-client/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Validate peer public keys during handshake in both trusted and untrusted connection flows ([#70](https://github.com/MetaMask/mobile-wallet-protocol/pull/70)) +### Fixed + +- Use constant-time comparison for OTP verification to prevent timing side-channel attacks + ## [0.2.2] ### Added diff --git a/packages/dapp-client/src/handlers/untrusted-connection-handler.ts b/packages/dapp-client/src/handlers/untrusted-connection-handler.ts index ea9564d..a70385f 100644 --- a/packages/dapp-client/src/handlers/untrusted-connection-handler.ts +++ b/packages/dapp-client/src/handlers/untrusted-connection-handler.ts @@ -1,4 +1,4 @@ -import { ClientState, ErrorCode, type HandshakeOfferPayload, type Session, SessionError, type SessionRequest } from "@metamask/mobile-wallet-protocol-core"; +import { ClientState, ErrorCode, type HandshakeOfferPayload, type Session, SessionError, type SessionRequest, timingSafeEqual } from "@metamask/mobile-wallet-protocol-core"; import { base64ToBytes } from "@metamask/utils"; import type { OtpRequiredPayload } from "../client"; import type { IConnectionHandler } from "../domain/connection-handler"; @@ -88,9 +88,10 @@ export class UntrustedConnectionHandler implements IConnectionHandler { return reject(new SessionError(ErrorCode.OTP_ENTRY_TIMEOUT, "The OTP has already expired.")); } + const expectedOtp = offer.otp; let attempts = 0; const submit = async (otp: string): Promise => { - if (otp !== offer.otp) { + if (!timingSafeEqual(otp, expectedOtp)) { attempts++; if (attempts >= this.otpAttempts) { reject(new SessionError(ErrorCode.OTP_MAX_ATTEMPTS_REACHED, "Maximum OTP attempts reached."));