🔒 [Fix unsafe timing-safe comparison with user input]

lib/mcp.js

@@ -182,11 +182,18 @@ export async function startMcpServer(options) {
             let authenticated = false;
 
             try {
-                if (queryToken && queryToken.length === token.length && crypto.timingSafeEqual(Buffer.from(queryToken), Buffer.from(token))) {
-                    authenticated = true;
-                } else if (authHeader && authHeader.startsWith('Bearer ')) {
+                const tBuffer = Buffer.from(token);
+                if (queryToken) {
+                    const qBuffer = Buffer.from(queryToken);
+                    if (qBuffer.length === tBuffer.length && crypto.timingSafeEqual(qBuffer, tBuffer)) {
+                        authenticated = true;
+                    }
+                }
+
+                if (!authenticated && authHeader && authHeader.startsWith('Bearer ')) {
                     const headerToken = authHeader.substring(7);
-                    if (headerToken.length === token.length && crypto.timingSafeEqual(Buffer.from(headerToken), Buffer.from(token))) {
+                    const hBuffer = Buffer.from(headerToken);
+                    if (hBuffer.length === tBuffer.length && crypto.timingSafeEqual(hBuffer, tBuffer)) {
                         authenticated = true;
                     }
                 }

tests/mcp-timing.test.js

@@ -0,0 +1,28 @@
+import crypto from 'crypto';
+
+describe('MCP Authentication Vulnerability', () => {
+    test('timingSafeEqual works properly with multibyte character attacks', () => {
+        const token = "regular_token_123";
+        // '£' has length 1 in javascript string, but 2 bytes in utf-8
+        // "regular_token_12£" has length 17, same as token
+        const queryToken = "regular_token_12£";
+
+        expect(queryToken.length).toBe(token.length);
+
+        let authenticated = false;
+        try {
+            const tBuffer = Buffer.from(token);
+            if (queryToken) {
+                const qBuffer = Buffer.from(queryToken);
+                if (qBuffer.length === tBuffer.length && crypto.timingSafeEqual(qBuffer, tBuffer)) {
+                    authenticated = true;
+                }
+            }
+        } catch(e) {
+            // this should not be reached!
+            expect(false).toBe(true);
+        }
+
+        expect(authenticated).toBe(false);
+    });
+});