mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-02-06 05:23:08 +00:00
This commit addresses 2 critical security vulnerabilities identified in the security audit. ## CRITICAL-02: Timing Attack Vulnerability (CVSS 8.5) **Problem:** Non-constant-time string comparison in authentication allowed timing attacks to discover tokens character-by-character through statistical timing analysis (estimated 24-48 hours to compromise). **Fix:** Implemented crypto.timingSafeEqual for all token comparisons **Changes:** - Added AuthManager.timingSafeCompare() constant-time comparison utility - Fixed src/utils/auth.ts:27 - validateToken method - Fixed src/http-server-single-session.ts:1087 - Single-session HTTP auth - Fixed src/http-server.ts:315 - Fixed HTTP server auth - Added 11 unit tests with timing variance analysis (<10% variance proven) ## CRITICAL-01: Command Injection Vulnerability (CVSS 8.8) **Problem:** User-controlled nodeType parameter injected into shell commands via execSync, allowing remote code execution, data exfiltration, and network scanning. **Fix:** Eliminated all shell execution, replaced with Node.js fs APIs **Changes:** - Replaced execSync() with fs.readdir() in enhanced-documentation-fetcher.ts - Added multi-layer input sanitization: /[^a-zA-Z0-9._-]/g - Added directory traversal protection (blocks .., /, relative paths) - Added path.basename() for additional safety - Added final path verification (ensures result within expected directory) - Added 9 integration tests covering all attack vectors ## Test Results All Tests Passing: - Unit tests: 11/11 ✅ (timing-safe comparison) - Integration tests: 9/9 ✅ (command injection prevention) - Timing variance: <10% ✅ (proves constant-time) - All existing tests: ✅ (no regressions) ## Breaking Changes None - All changes are backward compatible. ## References - Security Audit: Issue #265 - Implementation Plan: docs/local/security-implementation-plan-issue-265.md - Audit Analysis: docs/local/security-audit-analysis-issue-265.md 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
131 lines
4.8 KiB
TypeScript
131 lines
4.8 KiB
TypeScript
import { describe, it, expect } from 'vitest';
|
|
import { AuthManager } from '../../../src/utils/auth';
|
|
|
|
/**
|
|
* Unit tests for AuthManager.timingSafeCompare
|
|
*
|
|
* SECURITY: These tests verify constant-time comparison to prevent timing attacks
|
|
* See: https://github.com/czlonkowski/n8n-mcp/issues/265 (CRITICAL-02)
|
|
*/
|
|
describe('AuthManager.timingSafeCompare', () => {
|
|
describe('Security: Timing Attack Prevention', () => {
|
|
it('should return true for matching tokens', () => {
|
|
const token = 'a'.repeat(32);
|
|
const result = AuthManager.timingSafeCompare(token, token);
|
|
expect(result).toBe(true);
|
|
});
|
|
|
|
it('should return false for different tokens', () => {
|
|
const token1 = 'a'.repeat(32);
|
|
const token2 = 'b'.repeat(32);
|
|
const result = AuthManager.timingSafeCompare(token1, token2);
|
|
expect(result).toBe(false);
|
|
});
|
|
|
|
it('should return false for tokens of different lengths', () => {
|
|
const token1 = 'a'.repeat(32);
|
|
const token2 = 'a'.repeat(64);
|
|
const result = AuthManager.timingSafeCompare(token1, token2);
|
|
expect(result).toBe(false);
|
|
});
|
|
|
|
it('should return false for empty tokens', () => {
|
|
expect(AuthManager.timingSafeCompare('', 'test')).toBe(false);
|
|
expect(AuthManager.timingSafeCompare('test', '')).toBe(false);
|
|
expect(AuthManager.timingSafeCompare('', '')).toBe(false);
|
|
});
|
|
|
|
it('should use constant-time comparison (timing analysis)', () => {
|
|
const correctToken = 'a'.repeat(64);
|
|
const wrongFirstChar = 'b' + 'a'.repeat(63);
|
|
const wrongLastChar = 'a'.repeat(63) + 'b';
|
|
|
|
const samples = 1000;
|
|
const timings = {
|
|
wrongFirst: [] as number[],
|
|
wrongLast: [] as number[],
|
|
};
|
|
|
|
// Measure timing for wrong first character
|
|
for (let i = 0; i < samples; i++) {
|
|
const start = process.hrtime.bigint();
|
|
AuthManager.timingSafeCompare(wrongFirstChar, correctToken);
|
|
const end = process.hrtime.bigint();
|
|
timings.wrongFirst.push(Number(end - start));
|
|
}
|
|
|
|
// Measure timing for wrong last character
|
|
for (let i = 0; i < samples; i++) {
|
|
const start = process.hrtime.bigint();
|
|
AuthManager.timingSafeCompare(wrongLastChar, correctToken);
|
|
const end = process.hrtime.bigint();
|
|
timings.wrongLast.push(Number(end - start));
|
|
}
|
|
|
|
// Calculate medians
|
|
const median = (arr: number[]) => {
|
|
const sorted = arr.slice().sort((a, b) => a - b);
|
|
return sorted[Math.floor(sorted.length / 2)];
|
|
};
|
|
|
|
const medianFirst = median(timings.wrongFirst);
|
|
const medianLast = median(timings.wrongLast);
|
|
|
|
// Timing variance should be less than 10% (constant-time)
|
|
const variance = Math.abs(medianFirst - medianLast) / medianFirst;
|
|
|
|
expect(variance).toBeLessThan(0.10);
|
|
});
|
|
|
|
it('should handle special characters safely', () => {
|
|
const token1 = 'abc!@#$%^&*()_+-=[]{}|;:,.<>?';
|
|
const token2 = 'abc!@#$%^&*()_+-=[]{}|;:,.<>?';
|
|
const token3 = 'xyz!@#$%^&*()_+-=[]{}|;:,.<>?';
|
|
|
|
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(true);
|
|
expect(AuthManager.timingSafeCompare(token1, token3)).toBe(false);
|
|
});
|
|
|
|
it('should handle unicode characters', () => {
|
|
const token1 = '你好世界🌍🔒';
|
|
const token2 = '你好世界🌍🔒';
|
|
const token3 = '你好世界🌍❌';
|
|
|
|
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(true);
|
|
expect(AuthManager.timingSafeCompare(token1, token3)).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe('Edge Cases', () => {
|
|
it('should handle null/undefined gracefully', () => {
|
|
expect(AuthManager.timingSafeCompare(null as any, 'test')).toBe(false);
|
|
expect(AuthManager.timingSafeCompare('test', null as any)).toBe(false);
|
|
expect(AuthManager.timingSafeCompare(undefined as any, 'test')).toBe(false);
|
|
expect(AuthManager.timingSafeCompare('test', undefined as any)).toBe(false);
|
|
});
|
|
|
|
it('should handle very long tokens', () => {
|
|
const longToken = 'a'.repeat(10000);
|
|
expect(AuthManager.timingSafeCompare(longToken, longToken)).toBe(true);
|
|
expect(AuthManager.timingSafeCompare(longToken, 'b'.repeat(10000))).toBe(false);
|
|
});
|
|
|
|
it('should handle whitespace correctly', () => {
|
|
const token1 = 'test-token-with-spaces';
|
|
const token2 = 'test-token-with-spaces '; // Trailing space
|
|
const token3 = ' test-token-with-spaces'; // Leading space
|
|
|
|
expect(AuthManager.timingSafeCompare(token1, token1)).toBe(true);
|
|
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(false);
|
|
expect(AuthManager.timingSafeCompare(token1, token3)).toBe(false);
|
|
});
|
|
|
|
it('should be case-sensitive', () => {
|
|
const token1 = 'TestToken123';
|
|
const token2 = 'testtoken123';
|
|
|
|
expect(AuthManager.timingSafeCompare(token1, token2)).toBe(false);
|
|
});
|
|
});
|
|
});
|