diff --git a/lib/mcp.js b/lib/mcp.js index 577eba8..11b781b 100644 --- a/lib/mcp.js +++ b/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; } } diff --git a/tests/mcp-timing.test.js b/tests/mcp-timing.test.js new file mode 100644 index 0000000..154c84a --- /dev/null +++ b/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); + }); +});