Fixes RCE in uploads controllers

Author: DeanWard

app/Http/Controllers/EmailTemplatesController.php

@@ -5,6 +5,7 @@
 use Illuminate\Http\Request;
 use App\Models\Setting;
 use Illuminate\Support\Facades\Validator;
+use App\Utils\FileHelper;
 
 class EmailTemplatesController extends Controller
 {
@@ -94,12 +95,22 @@ public function update(Request $request)
   {
     $templates = $request->all();
     foreach ($templates as $template) {
+      // Validate template ID to prevent path traversal attacks
+      $templateId = $template['id'] ?? '';
+      if (!preg_match('/^[a-zA-Z0-9_-]+$/', $templateId)) {
+        return response()->json([
+          'status' => 'error',
+          'message' => 'Invalid template ID',
+          'data' => []
+        ], 400);
+      }
+      
       // Build dynamic validation rules based on original template
-      $originalPath = resource_path('views/emails/' . $template['id'] . '.twig');
+      $originalPath = resource_path('views/emails/' . $templateId . '.twig');
       if (!file_exists($originalPath)) {
         return response()->json([
           'status' => 'error',
-          'message' => 'Template not found: ' . $template['id'],
+          'message' => 'Template not found: ' . $templateId,
           'data' => []
         ], 404);
       }
@@ -144,9 +155,13 @@ public function update(Request $request)
 
   private function updateTemplate($template)
   {
-    $template_file_path = storage_path('templates/emails/' . $template['id'] . '.twig');
+    // Template ID should already be validated by update() method
+    // but add basename() as defense-in-depth
+    $safeId = basename($template['id']);
+    
+    $template_file_path = storage_path('templates/emails/' . $safeId . '.twig');
     Setting::updateOrCreate(
-      ['key' => 'email_subject_' . $template['id'] . '.twig'],
+      ['key' => 'email_subject_' . $safeId . '.twig'],
       ['value' => $template['subject'], 'group' => 'system.emails.subjects']
     );
     if (!file_exists(dirname($template_file_path))) {

app/Http/Controllers/TusdHooksController.php

@@ -422,12 +422,38 @@ protected function handleBundleUpload(string $uploadId, UploadSession $session,
             $fileIds = [];
             foreach ($manifest['files'] as $fileInfo) {
                 $filePath = $fileInfo['path'];
-                $extractedFilePath = $extractDir . '/' . $filePath;
+                
+                // Sanitize the file path to prevent path traversal attacks
+                // Remove ../ sequences and normalize path
+                $safePath = $this->sanitizeBundlePath($filePath);
+                if ($safePath === null) {
+                    Log::warning('tusd post-finish: Skipping file with dangerous path', [
+                        'upload_id' => $uploadId,
+                        'file_path' => $filePath
+                    ]);
+                    continue;
+                }
+                
+                $extractedFilePath = $extractDir . '/' . $safePath;
+                
+                // Verify the resolved path is within the extraction directory
+                $resolvedPath = realpath($extractedFilePath);
+                $resolvedExtractDir = realpath($extractDir);
+                
+                if ($resolvedPath === false || $resolvedExtractDir === false ||
+                    strpos($resolvedPath, $resolvedExtractDir) !== 0) {
+                    Log::warning('tusd post-finish: Path traversal attempt in bundle', [
+                        'upload_id' => $uploadId,
+                        'file_path' => $filePath,
+                        'resolved_path' => $resolvedPath
+                    ]);
+                    continue;
+                }
 
                 if (!file_exists($extractedFilePath)) {
                     Log::warning('tusd post-finish: Bundle file not found after extraction', [
                         'upload_id' => $uploadId,
-                        'file_path' => $filePath
+                        'file_path' => $safePath
                     ]);
                     continue;
                 }
@@ -440,7 +466,7 @@ protected function handleBundleUpload(string $uploadId, UploadSession $session,
                     'original_name' => $fileInfo['originalName'],
                     'type' => $fileInfo['type'] ?? 'application/octet-stream',
                     'size' => $fileInfo['size'],
-                    'temp_path' => 'uploads/' . $uploadId . '_extracted/' . $filePath
+                    'temp_path' => 'uploads/' . $uploadId . '_extracted/' . $safePath
                 ]);
 
                 $fileIds[] = $file->id;
@@ -531,6 +557,37 @@ protected function postTerminate(Request $request, array $payload)
         }
     }
 
+    /**
+     * Sanitize a bundle file path to prevent path traversal attacks
+     * Returns null if the path is dangerous and should be skipped
+     */
+    private function sanitizeBundlePath(string $path): ?string
+    {
+        // Normalize directory separators
+        $path = str_replace('\\', '/', $path);
+        
+        // Check for dangerous patterns before sanitization
+        if (strpos($path, '..') !== false) {
+            return null;
+        }
+        
+        // Remove leading slashes
+        $path = ltrim($path, '/');
+        
+        // Remove null bytes
+        $path = str_replace("\0", '', $path);
+        
+        // Clean up double slashes
+        $path = preg_replace('/\/+/', '/', $path);
+        
+        // If empty after sanitization, reject
+        if (empty($path)) {
+            return null;
+        }
+        
+        return $path;
+    }
+
     /**
      * Format bytes into human readable format
      */

app/Http/Controllers/UploadsController.php

@@ -279,13 +279,33 @@ public function createShareFromUploads(Request $request)
         $originalPath = $request->filePaths[$uploadId] ?? '';
         $originalPath = explode('/', $originalPath);
         $originalPath = implode('/', array_slice($originalPath, 0, -1));
+        
+        // Sanitize path to prevent directory traversal attacks
+        $originalPath = $this->sanitizePath($originalPath);
       }
 
       $destPath = $completePath . '/' . $originalPath;
-
+      
+      // Verify the resolved path is within the share directory
+      // Create parent directories first so realpath can resolve
       if (!file_exists($destPath)) {
         mkdir($destPath, 0777, true);
       }
+      $resolvedPath = realpath($destPath);
+      $resolvedSharePath = realpath($completePath);
+      
+      if ($resolvedPath === false || $resolvedSharePath === false || 
+          strpos($resolvedPath, $resolvedSharePath) !== 0) {
+        Log::warning('Path traversal attempt detected', [
+          'user_id' => $user->id,
+          'original_path' => $request->filePaths[$uploadId] ?? '',
+          'resolved_path' => $resolvedPath,
+          'share_path' => $resolvedSharePath
+        ]);
+        
+        // Clean up and skip this file
+        continue;
+      }
 
       // Use sanitized filename from database for file operations
       $sanitizedFilename = $file->name;
@@ -429,6 +449,33 @@ private function sendShareCreatedEmail(Share $share, $recipient)
     }
   }
 
+  /**
+   * Sanitize a file path to prevent directory traversal attacks
+   * Removes ../, ..\, and leading slashes
+   */
+  private function sanitizePath(string $path): string
+  {
+    // Normalize directory separators
+    $path = str_replace('\\', '/', $path);
+    
+    // Remove any ../ or ..\ sequences (handles encoded variants too)
+    $path = preg_replace('/\.\.[\\/\\\\]/', '', $path);
+    
+    // Also remove standalone .. 
+    $path = preg_replace('/\.\./', '', $path);
+    
+    // Remove leading slashes to prevent absolute paths
+    $path = ltrim($path, '/');
+    
+    // Remove any null bytes
+    $path = str_replace("\0", '', $path);
+    
+    // Clean up any double slashes that may have resulted
+    $path = preg_replace('/\/+/', '/', $path);
+    
+    return $path;
+  }
+
   /**
    * Recursively delete a directory and its contents
    */