Merge pull request #48 from ForgeOpus/copilot/fix-security-alerts-codeql

Fix path traversal, XSS, timing attack, and information disclosure vulnerabilities

Author: RETR0-OS

project/block_manager/utils/file_cleanup.py

@@ -23,9 +23,22 @@ def save_uploaded_file_temporarily(uploaded_file):
     upload_dir = Path(getattr(settings, 'TEMP_UPLOAD_DIR', '/tmp/visionforge_uploads'))
     upload_dir.mkdir(parents=True, exist_ok=True)
 
+    # Sanitize filename to prevent path traversal attacks
+    # Extract only the basename to remove any path components
+    original_name = os.path.basename(uploaded_file.name)
+    # Remove path separators that might have been missed (defense in depth)
+    safe_name = original_name.replace('/', '_').replace('\\', '_')
+    # Remove null bytes which can cause issues
+    safe_name = safe_name.replace('\x00', '')
+    
     timestamp = int(time.time())
-    safe_filename = f"{timestamp}_{uploaded_file.name}"
+    safe_filename = f"{timestamp}_{safe_name}"
     file_path = upload_dir / safe_filename
+    
+    # Verify the resolved path is within the upload directory (additional security check)
+    resolved_path = file_path.resolve()
+    if not str(resolved_path).startswith(str(upload_dir.resolve())):
+        raise ValueError("Invalid file path detected - potential path traversal attack")
 
     with open(file_path, 'wb+') as destination:
         for chunk in uploaded_file.chunks():

project/block_manager/views/architecture_views.py

@@ -399,10 +399,9 @@ def render_node_code(request):
             'format': 'class'
         })
     except Exception as e:
-        import traceback
-        traceback.print_exc()  # Log to console for debugging
+        logger.error(f"Error generating node code: {str(e)}", exc_info=True)
         return Response(
-            {'success': False, 'error': f'Error generating node code: {str(e)}'},
+            {'success': False, 'error': 'Error generating node code'},
             status=status.HTTP_500_INTERNAL_SERVER_ERROR
         )
 

project/block_manager/views/maintenance_views.py

@@ -3,6 +3,7 @@
 """
 import logging
 import time
+import hmac
 from pathlib import Path
 from django.http import JsonResponse
 from django.views.decorators.csrf import csrf_exempt
@@ -96,9 +97,9 @@ def trigger_file_cleanup(request):
         data = json.loads(request.body)
         provided_secret = data.get('secret', '')
 
-        # Verify secret token
+        # Verify secret token using constant-time comparison to prevent timing attacks
         expected_secret = settings.CLEANUP_SECRET_TOKEN
-        if not expected_secret or provided_secret != expected_secret:
+        if not expected_secret or not hmac.compare_digest(provided_secret, expected_secret):
             logger.warning(f"Unauthorized cleanup attempt from IP {request.META.get('REMOTE_ADDR')}")
             return JsonResponse({
                 'error': 'Unauthorized',
@@ -143,9 +144,9 @@ def get_upload_stats(request):
     try:
         provided_secret = request.GET.get('secret', '')
 
-        # Verify secret token
+        # Verify secret token using constant-time comparison to prevent timing attacks
         expected_secret = settings.CLEANUP_SECRET_TOKEN
-        if not expected_secret or provided_secret != expected_secret:
+        if not expected_secret or not hmac.compare_digest(provided_secret, expected_secret):
             logger.warning(f"Unauthorized stats access attempt from IP {request.META.get('REMOTE_ADDR')}")
             return JsonResponse({
                 'error': 'Unauthorized',

project/block_manager/views/validation_views.py

@@ -2,12 +2,14 @@
 from rest_framework.response import Response
 from rest_framework import status
 from rest_framework.permissions import AllowAny
-import traceback
+import logging
 
 from block_manager.serializers import SaveArchitectureSerializer
 from block_manager.services.validation import validate_architecture
 from block_manager.services.inference import infer_dimensions
 
+logger = logging.getLogger(__name__)
+
 
 @api_view(['POST'])
 @permission_classes([AllowAny])
@@ -43,13 +45,12 @@ def validate_model(request):
 
     except Exception as e:
         # Log the error for debugging
-        print(f"Validation error: {str(e)}")
-        traceback.print_exc()
+        logger.error(f"Validation error: {str(e)}", exc_info=True)
 
         return Response(
             {
                 'isValid': False,
-                'errors': [{'message': f'Server error: {str(e)}', 'type': 'error'}],
+                'errors': [{'message': 'Server error during validation', 'type': 'error'}],
             },
             status=status.HTTP_500_INTERNAL_SERVER_ERROR
         )

project/frontend/src/components/ui/chart.tsx

@@ -45,7 +45,9 @@ function ChartContainer({
   >["children"]
 }) {
   const uniqueId = useId()
-  const chartId = `chart-${id || uniqueId.replace(/:/g, "")}`
+  // Sanitize ID to prevent XSS - only allow alphanumeric, hyphens, and underscores
+  const baseId = (id || uniqueId).replace(/[^a-zA-Z0-9-_]/g, '')
+  const chartId = `chart-${baseId}`
 
   return (
     <ChartContext.Provider value={{ config }}>
@@ -76,19 +78,26 @@ const ChartStyle = ({ id, config }: { id: string; config: ChartConfig }) => {
     return null
   }
 
+  // Sanitize id to prevent CSS injection - only allow alphanumeric, hyphens, and underscores
+  const sanitizedId = id.replace(/[^a-zA-Z0-9-_]/g, '')
+
   return (
     <style
       dangerouslySetInnerHTML={{
         __html: Object.entries(THEMES)
           .map(
             ([theme, prefix]) => `
-${prefix} [data-chart=${id}] {
+${prefix} [data-chart=${sanitizedId}] {
 ${colorConfig
   .map(([key, itemConfig]) => {
+    // Sanitize key to prevent CSS injection
+    const sanitizedKey = key.replace(/[^a-zA-Z0-9-_]/g, '')
     const color =
       itemConfig.theme?.[theme as keyof typeof itemConfig.theme] ||
       itemConfig.color
-    return color ? `  --color-${key}: ${color};` : null
+    // Sanitize color value to prevent CSS injection
+    const sanitizedColor = color?.replace(/[<>'"]/g, '')
+    return sanitizedColor ? `  --color-${sanitizedKey}: ${sanitizedColor};` : null
   })
   .join("\n")}
 }