From f983269f930ea59c02018e7370ca38b502c4d65c Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Sun, 29 Mar 2026 18:14:18 -0400 Subject: [PATCH] Fix file upload security, bind_param mismatch, and cookie flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - upload_attachment.php: derive stored file extension from validated MIME type instead of user-supplied filename, preventing executable extension attacks (e.g. a PHP file renamed to evil.txt would now be stored as .txt) - CustomFieldModel.php: fix bind_param type string in updateDefinition() 'sssssiiiii' (10 chars) → 'sssssiiii' (9 chars) to match 9 SQL placeholders - RateLimitMiddleware.php: replace MD5 with SHA256 for rate limit file hashing - user_preferences.php: add httponly, secure, samesite=Lax flags to ticketsPerPage cookie to prevent XSS/CSRF cookie theft Co-Authored-By: Claude Sonnet 4.6 --- api/upload_attachment.php | 24 ++++++++++++++++++++---- api/user_preferences.php | 2 +- middleware/RateLimitMiddleware.php | 2 +- models/CustomFieldModel.php | 2 +- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/api/upload_attachment.php b/api/upload_attachment.php index 33a2452..0412148 100644 --- a/api/upload_attachment.php +++ b/api/upload_attachment.php @@ -151,10 +151,26 @@ if (!is_dir($ticketDir)) { } } -// Generate unique filename -$extension = pathinfo($file['name'], PATHINFO_EXTENSION); -$safeExtension = preg_replace('/[^a-zA-Z0-9]/', '', $extension); -$uniqueFilename = uniqid('att_', true) . ($safeExtension ? '.' . $safeExtension : ''); +// Derive extension from validated MIME type (never from user-supplied filename) +// This prevents executable extension attacks (e.g. evil.php disguised as text/plain) +$mimeToExt = [ + 'image/jpeg' => 'jpg', 'image/png' => 'png', + 'image/gif' => 'gif', 'image/webp' => 'webp', + 'application/pdf' => 'pdf', + 'text/plain' => 'txt', 'text/csv' => 'csv', + 'application/msword' => 'doc', + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' => 'docx', + 'application/vnd.ms-excel' => 'xls', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' => 'xlsx', + 'application/zip' => 'zip', + 'application/x-7z-compressed' => '7z', + 'application/x-tar' => 'tar', + 'application/gzip' => 'gz', + 'application/json' => 'json', + 'application/xml' => 'xml', +]; +$safeExtension = $mimeToExt[$mimeType] ?? 'bin'; +$uniqueFilename = uniqid('att_', true) . '.' . $safeExtension; $targetPath = $ticketDir . '/' . $uniqueFilename; // Move uploaded file diff --git a/api/user_preferences.php b/api/user_preferences.php index fcb3d2d..93151ea 100644 --- a/api/user_preferences.php +++ b/api/user_preferences.php @@ -43,7 +43,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') { if (!in_array($key, $validKeys)) continue; $prefsModel->setPreference($userId, $key, $value); if ($key === 'rows_per_page') { - setcookie('ticketsPerPage', $value, time() + (86400 * 365), '/'); + setcookie('ticketsPerPage', $value, ['expires' => time() + (86400 * 365), 'path' => '/', 'httponly' => true, 'secure' => true, 'samesite' => 'Lax']); } } echo json_encode(['success' => true]); diff --git a/middleware/RateLimitMiddleware.php b/middleware/RateLimitMiddleware.php index 2f3bf65..94065b9 100644 --- a/middleware/RateLimitMiddleware.php +++ b/middleware/RateLimitMiddleware.php @@ -64,7 +64,7 @@ class RateLimitMiddleware { $now = time(); // Create a hash of the IP for the filename (security + filesystem safety) - $ipHash = md5($ip . '_' . $type); + $ipHash = hash('sha256', $ip . '_' . $type); $filePath = self::getRateLimitDir() . '/' . $ipHash . '.json'; // Load existing rate data diff --git a/models/CustomFieldModel.php b/models/CustomFieldModel.php index 680c070..7312958 100644 --- a/models/CustomFieldModel.php +++ b/models/CustomFieldModel.php @@ -128,7 +128,7 @@ class CustomFieldModel { WHERE field_id = ?"; $stmt = $this->conn->prepare($sql); - $stmt->bind_param('sssssiiiii', + $stmt->bind_param('sssssiiii', $data['field_name'], $data['field_label'], $data['field_type'],