Fix critical security vulnerabilities - XSS, Cypher injection, memory limits

mvalancy · mvalancy · commit d12ee9f36f7d · 2025-08-23T03:01:46.000Z
SECURITY FIXES:
- XSS Prevention: Comprehensive input sanitization for all user data
- Cypher Injection: Pattern-based detection and parameterized queries
- Memory Protection: 512MB limit with monitoring and automatic shutdown
- Buffer Overflow: String length limits and validation
- Input Validation: Type checking and business logic enforcement

VULNERABILITY REMEDIATION:
- All XSS payloads now sanitized (script tags, event handlers, JS URLs)
- All Cypher injection attempts blocked (keywords, patterns, syntax)
- Memory exhaustion attacks prevented (10MB request limit, 512MB process limit)
- Node IDs validated against injection patterns
- Metadata objects recursively sanitized with depth limits

MONITORING:
- Real-time memory usage monitoring every 5 seconds
- Automatic garbage collection on memory pressure
- Graceful shutdown on memory limit exceeded
- Request-level memory validation
diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts
@@ -10,6 +10,7 @@ import {
 import neo4j from 'neo4j-driver';
 import { GraphService } from './services/graph-service.js';
 import { startHealthServer, recordAccess, recordClientConnection } from './health-server.js';
+import { startMemoryMonitoring, checkMemoryUsage } from './utils/memory-monitor.js';
 import {
   UpdatePrioritiesArgs,
   BulkUpdatePrioritiesArgs,
@@ -690,6 +691,9 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
 server.setRequestHandler(CallToolRequestSchema, async (request) => {
   const { name, arguments: args } = request.params;
   
+  // Check memory usage before processing request
+  checkMemoryUsage();
+  
   // Record MCP server access
   recordAccess();
 
@@ -811,6 +815,10 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
 async function main() {
   try {
     console.error('Initializing GraphDone MCP Server...');
+    
+    // Start memory monitoring first for security
+    startMemoryMonitoring();
+    
     await initializeDatabase();
     
     // Start health check server
diff --git a/packages/mcp-server/src/services/graph-service.ts b/packages/mcp-server/src/services/graph-service.ts
@@ -21,6 +21,16 @@ import {
   CapacityAnalysis,
   WorkloadPredictions
 } from '../types/graph.js';
+import {
+  sanitizeString,
+  sanitizeNodeId,
+  sanitizeMetadata,
+  sanitizeNodeType,
+  sanitizeNodeStatus,
+  sanitizePriority,
+  validateBulkOperation,
+  validateMemoryUsage
+} from '../utils/sanitizer.js';
 
 export interface PaginationInfo {
   total_count: number;
@@ -403,14 +413,23 @@ export class GraphService {
 
   async createNode(args: CreateNodeArgs) {
     return this.withSession(async (session) => {
-      const {
-        title = 'Untitled Node',
-        description = '',
-        type = 'TASK',
-        status = 'PROPOSED',
-        contributor_ids = [],
-        metadata = {}
-      } = args;
+      // Validate memory usage before processing
+      validateMemoryUsage(args, 10); // 10MB limit
+      
+      // Sanitize and validate all inputs
+      const title = sanitizeString(args.title || 'Untitled Node', 500);
+      const description = sanitizeString(args.description || '', 2000);
+      const type = sanitizeNodeType(args.type || 'TASK');
+      const status = sanitizeNodeStatus(args.status || 'PROPOSED');
+      const contributor_ids = Array.isArray(args.contributor_ids) ? 
+        args.contributor_ids.map(id => sanitizeNodeId(id)).slice(0, 50) : // Limit contributors
+        [];
+      const metadata = sanitizeMetadata(args.metadata || {});
+      
+      // Validate bulk operation if multiple contributors
+      if (contributor_ids.length > 0) {
+        validateBulkOperation(contributor_ids.length, 50);
+      }
 
       const id = `node_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`;
       const now = new Date().toISOString();
@@ -474,40 +493,50 @@ export class GraphService {
 
   async updateNode(args: UpdateNodeArgs) {
     return this.withSession(async (session) => {
+      // Validate memory usage before processing
+      validateMemoryUsage(args, 10); // 10MB limit
+      
       const { node_id, ...updates } = args;
       
       if (!node_id) {
         return {
           content: [{
             type: 'text',
-            text: 'node_id is required for updating a node'
+            text: JSON.stringify({ error: 'node_id is required for updating a node' })
           }],
           isError: true
         };
       }
       
+      // Sanitize node ID
+      const sanitizedNodeId = sanitizeNodeId(node_id);
+      
       // Build the SET clause dynamically based on provided fields
       const setClause: string[] = ['n.updatedAt = $now'];
       const params: Neo4jParams = { 
-        node_id, 
+        node_id: sanitizedNodeId, 
         now: new Date().toISOString() 
       };
 
       if (updates.title !== undefined) {
         setClause.push('n.title = $title');
-        params.title = updates.title;
+        params.title = sanitizeString(updates.title, 500);
       }
       if (updates.description !== undefined) {
         setClause.push('n.description = $description');
-        params.description = updates.description;
+        params.description = sanitizeString(updates.description, 2000);
       }
       if (updates.status !== undefined) {
         setClause.push('n.status = $status');
-        params.status = updates.status;
+        params.status = sanitizeNodeStatus(updates.status);
+      }
+      if (updates.type !== undefined) {
+        setClause.push('n.type = $type');
+        params.type = sanitizeNodeType(updates.type);
       }
       if (updates.metadata !== undefined) {
         setClause.push('n.metadata = $metadata');
-        params.metadata = JSON.stringify(updates.metadata);
+        params.metadata = JSON.stringify(sanitizeMetadata(updates.metadata));
       }
 
       const query = `
@@ -717,9 +746,12 @@ export class GraphService {
     return this.withSession(async (session) => {
       const { node_id, relationships_limit = 20, relationships_offset = 0 } = args;
       
-      // Ensure all numeric values are properly converted to integers
-      const relationshipsLimitInt = typeof relationships_limit === 'number' ? Math.floor(relationships_limit) : parseInt(String(relationships_limit), 10) || 20;
-      const relationshipsOffsetInt = typeof relationships_offset === 'number' ? Math.floor(relationships_offset) : parseInt(String(relationships_offset), 10) || 0;
+      // Sanitize node ID input
+      const sanitizedNodeId = sanitizeNodeId(node_id);
+      
+      // Ensure all numeric values are properly converted to integers with limits
+      const relationshipsLimitInt = Math.min(Math.max(1, Math.floor(Number(relationships_limit) || 20)), 100); // Max 100 relationships
+      const relationshipsOffsetInt = Math.max(0, Math.floor(Number(relationships_offset) || 0));
 
       // First get the node and basic info
       const nodeQuery = `
@@ -746,11 +778,11 @@ export class GraphService {
         LIMIT $relationships_limit
       `;
 
-      // Execute queries
-      const nodeResult = await session.run(nodeQuery, { node_id });
-      const relCountResult = await session.run(relCountQuery, { node_id });
+      // Execute queries with sanitized node ID
+      const nodeResult = await session.run(nodeQuery, { node_id: sanitizedNodeId });
+      const relCountResult = await session.run(relCountQuery, { node_id: sanitizedNodeId });
       const relationshipsResult = await session.run(relationshipsQuery, { 
-        node_id, 
+        node_id: sanitizedNodeId, 
         relationships_offset: int(relationshipsOffsetInt),
         relationships_limit: int(relationshipsLimitInt)
       });
@@ -761,7 +793,7 @@ export class GraphService {
             type: 'text',
             text: JSON.stringify({
               error: 'Node not found',
-              node_id
+              node_id: sanitizedNodeId
             }, null, 2)
           }],
           isError: true
@@ -986,22 +1018,34 @@ export class GraphService {
         recalculate_computed = true 
       } = args;
 
+      // Sanitize node ID
+      const sanitizedNodeId = sanitizeNodeId(node_id);
+
       const updates: string[] = [];
-      const params: Neo4jParams = { node_id };
+      const params: Neo4jParams = { node_id: sanitizedNodeId };
 
       if (priority_executive !== undefined) {
-        updates.push('n.priorityExec = $priority_executive');
-        params.priority_executive = Math.max(0, Math.min(1, priority_executive));
+        const sanitizedPriority = sanitizePriority(priority_executive);
+        if (sanitizedPriority !== null) {
+          updates.push('n.priorityExec = $priority_executive');
+          params.priority_executive = sanitizedPriority;
+        }
       }
 
       if (priority_individual !== undefined) {
-        updates.push('n.priorityIndiv = $priority_individual');
-        params.priority_individual = Math.max(0, Math.min(1, priority_individual));
+        const sanitizedPriority = sanitizePriority(priority_individual);
+        if (sanitizedPriority !== null) {
+          updates.push('n.priorityIndiv = $priority_individual');
+          params.priority_individual = sanitizedPriority;
+        }
       }
 
       if (priority_community !== undefined) {
-        updates.push('n.priorityComm = $priority_community');
-        params.priority_community = Math.max(0, Math.min(1, priority_community));
+        const sanitizedPriority = sanitizePriority(priority_community);
+        if (sanitizedPriority !== null) {
+          updates.push('n.priorityComm = $priority_community');
+          params.priority_community = sanitizedPriority;
+        }
       }
 
       if (recalculate_computed) {
diff --git a/packages/mcp-server/src/utils/memory-monitor.ts b/packages/mcp-server/src/utils/memory-monitor.ts
@@ -0,0 +1,153 @@
+/**
+ * Memory monitoring and protection utilities
+ */
+
+let memoryWarningCount = 0;
+let lastMemoryCheck = 0;
+const MEMORY_CHECK_INTERVAL = 5000; // 5 seconds
+const MEMORY_LIMIT_MB = 512; // 512MB limit
+const MEMORY_WARNING_MB = 256; // 256MB warning threshold
+
+/**
+ * Check current memory usage and enforce limits
+ */
+export function checkMemoryUsage(): void {
+  const now = Date.now();
+  
+  // Don't check too frequently to avoid performance impact
+  if (now - lastMemoryCheck < MEMORY_CHECK_INTERVAL) {
+    return;
+  }
+  
+  lastMemoryCheck = now;
+  
+  const memoryUsage = process.memoryUsage();
+  const heapUsedMB = memoryUsage.heapUsed / (1024 * 1024);
+  const totalMemoryMB = memoryUsage.rss / (1024 * 1024);
+  
+  // Enforce hard memory limit
+  if (heapUsedMB > MEMORY_LIMIT_MB) {
+    console.error(`🚨 MEMORY LIMIT EXCEEDED: ${heapUsedMB.toFixed(2)}MB > ${MEMORY_LIMIT_MB}MB`);
+    
+    // Force garbage collection if available
+    if (global.gc) {
+      console.log('🗑️ Forcing garbage collection...');
+      global.gc();
+      
+      // Check memory again after GC
+      const afterGC = process.memoryUsage().heapUsed / (1024 * 1024);
+      if (afterGC > MEMORY_LIMIT_MB) {
+        throw new Error(`Memory limit exceeded: ${afterGC.toFixed(2)}MB. Server shutting down to prevent system instability.`);
+      }
+    } else {
+      throw new Error(`Memory limit exceeded: ${heapUsedMB.toFixed(2)}MB. Server shutting down to prevent system instability.`);
+    }
+  }
+  
+  // Warning for high memory usage
+  if (heapUsedMB > MEMORY_WARNING_MB) {
+    memoryWarningCount++;
+    
+    if (memoryWarningCount % 5 === 0) { // Log every 5th warning to avoid spam
+      console.warn(`⚠️ HIGH MEMORY USAGE: ${heapUsedMB.toFixed(2)}MB (Warning #${memoryWarningCount})`);
+      console.warn(`RSS: ${totalMemoryMB.toFixed(2)}MB, External: ${(memoryUsage.external / (1024 * 1024)).toFixed(2)}MB`);
+    }
+  } else {
+    memoryWarningCount = 0; // Reset warning count when memory is back to normal
+  }
+}
+
+/**
+ * Validate that an operation won't exceed memory limits
+ */
+export function validateOperationMemory(estimatedSizeMB: number): void {
+  const currentMemoryMB = process.memoryUsage().heapUsed / (1024 * 1024);
+  const projectedMemoryMB = currentMemoryMB + estimatedSizeMB;
+  
+  if (projectedMemoryMB > MEMORY_LIMIT_MB) {
+    throw new Error(`Operation would exceed memory limit: ${projectedMemoryMB.toFixed(2)}MB > ${MEMORY_LIMIT_MB}MB`);
+  }
+  
+  if (projectedMemoryMB > MEMORY_WARNING_MB) {
+    console.warn(`⚠️ Operation may cause high memory usage: ${projectedMemoryMB.toFixed(2)}MB`);
+  }
+}
+
+/**
+ * Get current memory statistics
+ */
+export function getMemoryStats() {
+  const usage = process.memoryUsage();
+  
+  return {
+    heapUsed: Math.round(usage.heapUsed / (1024 * 1024) * 100) / 100, // MB
+    heapTotal: Math.round(usage.heapTotal / (1024 * 1024) * 100) / 100, // MB
+    rss: Math.round(usage.rss / (1024 * 1024) * 100) / 100, // MB
+    external: Math.round(usage.external / (1024 * 1024) * 100) / 100, // MB
+    limit: MEMORY_LIMIT_MB,
+    warning: MEMORY_WARNING_MB,
+    warningCount: memoryWarningCount
+  };
+}
+
+/**
+ * Start periodic memory monitoring
+ */
+export function startMemoryMonitoring(): void {
+  // Check memory immediately
+  checkMemoryUsage();
+  
+  // Set up periodic monitoring
+  const monitoringInterval = setInterval(() => {
+    try {
+      checkMemoryUsage();
+    } catch (error) {
+      console.error('💥 Memory monitoring failed:', error);
+      clearInterval(monitoringInterval);
+      
+      // Critical memory issue - attempt graceful shutdown
+      console.error('🚨 Server shutting down due to memory issues');
+      process.exit(1);
+    }
+  }, MEMORY_CHECK_INTERVAL);
+  
+  // Clean up on process exit
+  process.on('SIGINT', () => {
+    clearInterval(monitoringInterval);
+  });
+  
+  process.on('SIGTERM', () => {
+    clearInterval(monitoringInterval);
+  });
+  
+  console.log(`🛡️ Memory monitoring started (Limit: ${MEMORY_LIMIT_MB}MB, Warning: ${MEMORY_WARNING_MB}MB)`);
+}
+
+/**
+ * Memory-safe JSON parsing with size limits
+ */
+export function safeJsonParse(jsonString: string, maxSizeMB: number = 10): any {
+  const sizeBytes = Buffer.byteLength(jsonString, 'utf8');
+  const sizeMB = sizeBytes / (1024 * 1024);
+  
+  if (sizeMB > maxSizeMB) {
+    throw new Error(`JSON size limit exceeded: ${sizeMB.toFixed(2)}MB > ${maxSizeMB}MB`);
+  }
+  
+  return JSON.parse(jsonString);
+}
+
+/**
+ * Memory-safe JSON stringification with size limits
+ */
+export function safeJsonStringify(obj: any, maxSizeMB: number = 10): string {
+  const jsonString = JSON.stringify(obj);
+  const sizeBytes = Buffer.byteLength(jsonString, 'utf8');
+  const sizeMB = sizeBytes / (1024 * 1024);
+  
+  if (sizeMB > maxSizeMB) {
+    throw new Error(`JSON output size limit exceeded: ${sizeMB.toFixed(2)}MB > ${maxSizeMB}MB`);
+  }
+  
+  return jsonString;
+}
diff --git a/packages/mcp-server/src/utils/sanitizer.ts b/packages/mcp-server/src/utils/sanitizer.ts
