fix: use timing-safe comparison for OTP verification (WAPI-1128)

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";

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);
+	});
+});

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;
+}

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

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<void> => {
-				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."));