From 5b2a2c271ee8a7a286839ceffc0771fd6e8fce0b Mon Sep 17 00:00:00 2001 From: Jared Vititoe Date: Fri, 30 Jan 2026 18:51:16 -0500 Subject: [PATCH] Add security logging, domain validation, and output helpers - Add authentication failure logging to AuthMiddleware (session expiry, access denied, unauthenticated access attempts) - Add UrlHelper for secure URL generation with host validation against configurable ALLOWED_HOSTS whitelist - Add OutputHelper with consistent XSS-safe escaping functions (h, attr, json, url, css, truncate, date, cssClass) - Add validation to AuditLogModel query parameters (pagination limits, date format validation, action/entity type validation, IP sanitization) - Add APP_DOMAIN and ALLOWED_HOSTS configuration options Co-Authored-By: Claude Opus 4.5 --- api/update_ticket.php | 7 +- config/config.php | 8 ++ controllers/TicketController.php | 7 +- create_ticket_api.php | 6 +- helpers/OutputHelper.php | 198 +++++++++++++++++++++++++++++++ helpers/UrlHelper.php | 99 ++++++++++++++++ middleware/AuthMiddleware.php | 53 +++++++++ models/AuditLogModel.php | 192 +++++++++++++++++++++++++----- 8 files changed, 528 insertions(+), 42 deletions(-) create mode 100644 helpers/OutputHelper.php create mode 100644 helpers/UrlHelper.php diff --git a/api/update_ticket.php b/api/update_ticket.php index 35ac31b..482e38b 100644 --- a/api/update_ticket.php +++ b/api/update_ticket.php @@ -15,6 +15,7 @@ try { $configPath = dirname(__DIR__) . '/config/config.php'; require_once $configPath; require_once dirname(__DIR__) . '/helpers/Database.php'; + require_once dirname(__DIR__) . '/helpers/UrlHelper.php'; // Load environment variables (for Discord webhook) $envPath = dirname(__DIR__) . '/.env'; @@ -220,10 +221,8 @@ try { return; } - // Create ticket URL - $protocol = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off') ? 'https' : 'http'; - $host = $_SERVER['HTTP_HOST'] ?? 't.lotusguild.org'; - $ticketUrl = "{$protocol}://{$host}/ticket/{$ticketId}"; + // Create ticket URL using validated host + $ticketUrl = UrlHelper::ticketUrl($ticketId); // Determine embed color based on priority $colors = [ diff --git a/config/config.php b/config/config.php index 36873d5..4cbde2a 100644 --- a/config/config.php +++ b/config/config.php @@ -31,6 +31,14 @@ $GLOBALS['config'] = [ 'ASSETS_URL' => '/assets', // Assets URL 'API_URL' => '/api', // API URL + // Domain settings for external integrations (webhooks, links, etc.) + // Set APP_DOMAIN in .env to override + 'APP_DOMAIN' => $envVars['APP_DOMAIN'] ?? null, + // Allowed hosts for HTTP_HOST validation (comma-separated in .env) + 'ALLOWED_HOSTS' => array_filter(array_map('trim', + explode(',', $envVars['ALLOWED_HOSTS'] ?? 'localhost,127.0.0.1') + )), + // Session settings 'SESSION_TIMEOUT' => 3600, // 1 hour in seconds 'SESSION_REGENERATE_INTERVAL' => 300, // Regenerate session ID every 5 minutes diff --git a/controllers/TicketController.php b/controllers/TicketController.php index 99d27f8..904aa3b 100644 --- a/controllers/TicketController.php +++ b/controllers/TicketController.php @@ -6,6 +6,7 @@ require_once dirname(__DIR__) . '/models/AuditLogModel.php'; require_once dirname(__DIR__) . '/models/UserModel.php'; require_once dirname(__DIR__) . '/models/WorkflowModel.php'; require_once dirname(__DIR__) . '/models/TemplateModel.php'; +require_once dirname(__DIR__) . '/helpers/UrlHelper.php'; class TicketController { private $ticketModel; @@ -218,10 +219,8 @@ class TicketController { $webhookUrl = $this->envVars['DISCORD_WEBHOOK_URL']; - // Create ticket URL - $protocol = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off') ? 'https' : 'http'; - $host = $_SERVER['HTTP_HOST'] ?? 't.lotusguild.org'; - $ticketUrl = "{$protocol}://{$host}/ticket/{$ticketId}"; + // Create ticket URL using validated host + $ticketUrl = UrlHelper::ticketUrl($ticketId); // Map priorities to Discord colors (matching API endpoint) $priorityColors = [ diff --git a/create_ticket_api.php b/create_ticket_api.php index 221437b..e254b0a 100644 --- a/create_ticket_api.php +++ b/create_ticket_api.php @@ -52,6 +52,7 @@ if ($conn->connect_error) { // Authenticate via API key require_once __DIR__ . '/middleware/ApiKeyAuth.php'; require_once __DIR__ . '/models/AuditLogModel.php'; +require_once __DIR__ . '/helpers/UrlHelper.php'; $apiKeyAuth = new ApiKeyAuth($conn); @@ -245,9 +246,8 @@ if (isset($envVars['DISCORD_WEBHOOK_URL']) && !empty($envVars['DISCORD_WEBHOOK_U "5" => "P5 - Info" ]; - $protocol = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off') ? 'https' : 'http'; - $host = $_SERVER['HTTP_HOST'] ?? 't.lotusguild.org'; - $ticketUrl = "{$protocol}://{$host}/ticket/{$ticket_id}"; + // Create ticket URL using validated host + $ticketUrl = UrlHelper::ticketUrl($ticket_id); // Extract hostname from title for cleaner display preg_match('/^\[([^\]]+)\]/', $title, $hostnameMatch); diff --git a/helpers/OutputHelper.php b/helpers/OutputHelper.php new file mode 100644 index 0000000..0dcde49 --- /dev/null +++ b/helpers/OutputHelper.php @@ -0,0 +1,198 @@ +

+ * + * @param string|null $string The string to escape + * @param int $flags htmlspecialchars flags (default: ENT_QUOTES | ENT_HTML5) + * @return string Escaped string + */ + public static function h(?string $string, int $flags = ENT_QUOTES | ENT_HTML5): string { + if ($string === null) { + return ''; + } + return htmlspecialchars($string, $flags, 'UTF-8'); + } + + /** + * Escape string for HTML attribute context + * + * Use for values inside HTML attributes. + * Example: + * + * @param string|null $string The string to escape + * @return string Escaped string + */ + public static function attr(?string $string): string { + if ($string === null) { + return ''; + } + // More aggressive escaping for attribute context + return htmlspecialchars($string, ENT_QUOTES | ENT_HTML5 | ENT_SUBSTITUTE, 'UTF-8'); + } + + /** + * Encode data as JSON for JavaScript context + * + * Use when embedding data in JavaScript. + * Example: + * + * @param mixed $data The data to encode + * @param int $flags json_encode flags + * @return string JSON encoded string (safe for script context) + */ + public static function json($data, int $flags = 0): string { + // Use HEX encoding for safety in HTML context + $safeFlags = JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP | $flags; + return json_encode($data, $safeFlags); + } + + /** + * URL encode a string + * + * Use for values in URL query strings. + * Example: + * + * @param string|null $string The string to encode + * @return string URL encoded string + */ + public static function url(?string $string): string { + if ($string === null) { + return ''; + } + return rawurlencode($string); + } + + /** + * Escape for CSS context + * + * Use for values in inline CSS. + * Example:
+ * + * @param string|null $string The string to escape + * @return string Escaped string (only allows safe characters) + */ + public static function css(?string $string): string { + if ($string === null) { + return ''; + } + // Only allow alphanumeric, hyphens, underscores, spaces, and common CSS values + if (!preg_match('/^[a-zA-Z0-9_\-\s#.,()%]+$/', $string)) { + return ''; + } + return $string; + } + + /** + * Format a number safely + * + * Ensures output is always a valid number. + * + * @param mixed $number The number to format + * @param int $decimals Number of decimal places + * @return string Formatted number + */ + public static function number($number, int $decimals = 0): string { + return number_format((float)$number, $decimals, '.', ','); + } + + /** + * Format an integer safely + * + * @param mixed $value The value to format + * @return int Integer value + */ + public static function int($value): int { + return (int)$value; + } + + /** + * Truncate string with ellipsis + * + * @param string|null $string The string to truncate + * @param int $length Maximum length + * @param string $suffix Suffix to add if truncated + * @return string Truncated and escaped string + */ + public static function truncate(?string $string, int $length = 100, string $suffix = '...'): string { + if ($string === null) { + return ''; + } + + if (mb_strlen($string, 'UTF-8') <= $length) { + return self::h($string); + } + + return self::h(mb_substr($string, 0, $length, 'UTF-8')) . self::h($suffix); + } + + /** + * Format a date safely + * + * @param string|int|null $date Date string, timestamp, or null + * @param string $format PHP date format + * @return string Formatted date + */ + public static function date($date, string $format = 'Y-m-d H:i:s'): string { + if ($date === null || $date === '') { + return ''; + } + + if (is_numeric($date)) { + return date($format, (int)$date); + } + + $timestamp = strtotime($date); + if ($timestamp === false) { + return ''; + } + + return date($format, $timestamp); + } + + /** + * Check if a string is safe for use as a CSS class name + * + * @param string $class The class name to validate + * @return bool True if safe + */ + public static function isValidCssClass(string $class): bool { + return preg_match('/^[a-zA-Z_][a-zA-Z0-9_-]*$/', $class) === 1; + } + + /** + * Sanitize CSS class name(s) + * + * @param string|null $classes Space-separated class names + * @return string Sanitized class names + */ + public static function cssClass(?string $classes): string { + if ($classes === null || $classes === '') { + return ''; + } + + $classList = explode(' ', $classes); + $validClasses = array_filter($classList, [self::class, 'isValidCssClass']); + + return implode(' ', $validClasses); + } +} + +/** + * Shorthand function for HTML escaping + * + * @param string|null $string The string to escape + * @return string Escaped string + */ +function h(?string $string): string { + return OutputHelper::h($string); +} diff --git a/helpers/UrlHelper.php b/helpers/UrlHelper.php new file mode 100644 index 0000000..b2614d1 --- /dev/null +++ b/helpers/UrlHelper.php @@ -0,0 +1,99 @@ +userModel = new UserModel($conn); } + /** + * Log security event for authentication failures + * + * @param string $event Event type (e.g., 'auth_required', 'access_denied', 'session_expired') + * @param array $context Additional context data + */ + private function logSecurityEvent(string $event, array $context = []): void { + $logData = [ + 'event' => $event, + 'ip' => $_SERVER['REMOTE_ADDR'] ?? 'unknown', + 'forwarded_for' => $_SERVER['HTTP_X_FORWARDED_FOR'] ?? null, + 'user_agent' => $_SERVER['HTTP_USER_AGENT'] ?? 'unknown', + 'request_uri' => $_SERVER['REQUEST_URI'] ?? 'unknown', + 'request_method' => $_SERVER['REQUEST_METHOD'] ?? 'unknown', + 'timestamp' => date('c') + ]; + + // Merge additional context + $logData = array_merge($logData, $context); + + // Remove null values for cleaner logs + $logData = array_filter($logData, fn($v) => $v !== null); + + // Format log message + $message = sprintf( + "[SECURITY] %s: %s", + strtoupper($event), + json_encode($logData, JSON_UNESCAPED_SLASHES) + ); + + error_log($message); + } + /** * Authenticate user from Authelia forward auth headers * @@ -37,6 +70,13 @@ class AuthMiddleware { if (isset($_SESSION['user']) && isset($_SESSION['user']['user_id'])) { // Verify session hasn't expired (5 hour timeout) if (isset($_SESSION['last_activity']) && (time() - $_SESSION['last_activity'] > 18000)) { + // Log session expiration + $this->logSecurityEvent('session_expired', [ + 'username' => $_SESSION['user']['username'] ?? 'unknown', + 'user_id' => $_SESSION['user']['user_id'] ?? null, + 'session_age_seconds' => time() - $_SESSION['last_activity'] + ]); + // Session expired, clear it session_unset(); session_destroy(); @@ -123,6 +163,11 @@ class AuthMiddleware { * Redirect to Authelia login */ private function redirectToAuth() { + // Log unauthenticated access attempt + $this->logSecurityEvent('auth_required', [ + 'reason' => 'no_auth_headers' + ]); + // Redirect to the auth endpoint (Authelia will handle the redirect back) header('HTTP/1.1 401 Unauthorized'); echo ' @@ -187,6 +232,14 @@ class AuthMiddleware { * @param string $groups User groups */ private function showAccessDenied($username, $groups) { + // Log access denied event with user details + $this->logSecurityEvent('access_denied', [ + 'username' => $username, + 'groups' => $groups ?: 'none', + 'required_groups' => 'admin,employee', + 'reason' => 'insufficient_group_membership' + ]); + header('HTTP/1.1 403 Forbidden'); echo ' diff --git a/models/AuditLogModel.php b/models/AuditLogModel.php index 888a341..565041c 100644 --- a/models/AuditLogModel.php +++ b/models/AuditLogModel.php @@ -5,10 +5,92 @@ class AuditLogModel { private $conn; + /** @var int Maximum allowed limit for pagination */ + private const MAX_LIMIT = 1000; + + /** @var int Default limit for pagination */ + private const DEFAULT_LIMIT = 100; + + /** @var array Allowed action types for filtering */ + private const VALID_ACTION_TYPES = [ + 'create', 'update', 'delete', 'view', 'security_event', + 'login', 'logout', 'assign', 'comment', 'bulk_update' + ]; + + /** @var array Allowed entity types for filtering */ + private const VALID_ENTITY_TYPES = [ + 'ticket', 'comment', 'user', 'api_key', 'security', + 'template', 'attachment', 'group' + ]; + public function __construct($conn) { $this->conn = $conn; } + /** + * Validate and sanitize pagination limit + * + * @param int $limit Requested limit + * @return int Validated limit + */ + private function validateLimit(int $limit): int { + if ($limit < 1) { + return self::DEFAULT_LIMIT; + } + return min($limit, self::MAX_LIMIT); + } + + /** + * Validate and sanitize pagination offset + * + * @param int $offset Requested offset + * @return int Validated offset (non-negative) + */ + private function validateOffset(int $offset): int { + return max(0, $offset); + } + + /** + * Validate date format (YYYY-MM-DD) + * + * @param string $date Date string + * @return string|null Validated date or null if invalid + */ + private function validateDate(string $date): ?string { + // Check format + if (!preg_match('/^\d{4}-\d{2}-\d{2}$/', $date)) { + return null; + } + + // Verify it's a valid date + $parts = explode('-', $date); + if (!checkdate((int)$parts[1], (int)$parts[2], (int)$parts[0])) { + return null; + } + + return $date; + } + + /** + * Validate action type + * + * @param string $actionType Action type to validate + * @return bool True if valid + */ + private function isValidActionType(string $actionType): bool { + return in_array($actionType, self::VALID_ACTION_TYPES, true); + } + + /** + * Validate entity type + * + * @param string $entityType Entity type to validate + * @return bool True if valid + */ + private function isValidEntityType(string $entityType): bool { + return in_array($entityType, self::VALID_ENTITY_TYPES, true); + } + /** * Log an action to the audit trail * @@ -53,6 +135,8 @@ class AuditLogModel { * @return array Array of audit log records */ public function getLogsByEntity($entityType, $entityId, $limit = 100) { + $limit = $this->validateLimit((int)$limit); + $stmt = $this->conn->prepare( "SELECT al.*, u.username, u.display_name FROM audit_log al @@ -86,6 +170,9 @@ class AuditLogModel { * @return array Array of audit log records */ public function getLogsByUser($userId, $limit = 100) { + $limit = $this->validateLimit((int)$limit); + $userId = max(0, (int)$userId); + $stmt = $this->conn->prepare( "SELECT al.*, u.username, u.display_name FROM audit_log al @@ -119,6 +206,9 @@ class AuditLogModel { * @return array Array of audit log records */ public function getRecentLogs($limit = 50, $offset = 0) { + $limit = $this->validateLimit((int)$limit); + $offset = $this->validateOffset((int)$offset); + $stmt = $this->conn->prepare( "SELECT al.*, u.username, u.display_name FROM audit_log al @@ -151,6 +241,13 @@ class AuditLogModel { * @return array Array of audit log records */ public function getLogsByAction($actionType, $limit = 100) { + $limit = $this->validateLimit((int)$limit); + + // Validate action type to prevent unexpected queries + if (!$this->isValidActionType($actionType)) { + return []; + } + $stmt = $this->conn->prepare( "SELECT al.*, u.username, u.display_name FROM audit_log al @@ -370,6 +467,9 @@ class AuditLogModel { * @return array Security events */ public function getSecurityEvents($limit = 100, $offset = 0) { + $limit = $this->validateLimit((int)$limit); + $offset = $this->validateOffset((int)$offset); + $stmt = $this->conn->prepare( "SELECT al.*, u.username, u.display_name FROM audit_log al @@ -435,59 +535,89 @@ class AuditLogModel { * @return array Array containing logs and total count */ public function getFilteredLogs($filters = [], $limit = 50, $offset = 0) { + // Validate pagination parameters + $limit = $this->validateLimit((int)$limit); + $offset = $this->validateOffset((int)$offset); + $whereConditions = []; $params = []; $paramTypes = ''; - // Action type filter + // Action type filter - validate each action type if (!empty($filters['action_type'])) { - $actions = explode(',', $filters['action_type']); - $placeholders = str_repeat('?,', count($actions) - 1) . '?'; - $whereConditions[] = "al.action_type IN ($placeholders)"; - $params = array_merge($params, $actions); - $paramTypes .= str_repeat('s', count($actions)); + $actions = array_filter( + array_map('trim', explode(',', $filters['action_type'])), + fn($action) => $this->isValidActionType($action) + ); + if (!empty($actions)) { + $placeholders = str_repeat('?,', count($actions) - 1) . '?'; + $whereConditions[] = "al.action_type IN ($placeholders)"; + $params = array_merge($params, array_values($actions)); + $paramTypes .= str_repeat('s', count($actions)); + } } - // Entity type filter + // Entity type filter - validate each entity type if (!empty($filters['entity_type'])) { - $entities = explode(',', $filters['entity_type']); - $placeholders = str_repeat('?,', count($entities) - 1) . '?'; - $whereConditions[] = "al.entity_type IN ($placeholders)"; - $params = array_merge($params, $entities); - $paramTypes .= str_repeat('s', count($entities)); + $entities = array_filter( + array_map('trim', explode(',', $filters['entity_type'])), + fn($entity) => $this->isValidEntityType($entity) + ); + if (!empty($entities)) { + $placeholders = str_repeat('?,', count($entities) - 1) . '?'; + $whereConditions[] = "al.entity_type IN ($placeholders)"; + $params = array_merge($params, array_values($entities)); + $paramTypes .= str_repeat('s', count($entities)); + } } - // User filter + // User filter - validate as positive integer if (!empty($filters['user_id'])) { - $whereConditions[] = "al.user_id = ?"; - $params[] = (int)$filters['user_id']; - $paramTypes .= 'i'; + $userId = (int)$filters['user_id']; + if ($userId > 0) { + $whereConditions[] = "al.user_id = ?"; + $params[] = $userId; + $paramTypes .= 'i'; + } } - // Entity ID filter (for specific ticket/comment) + // Entity ID filter - sanitize (alphanumeric and dashes only) if (!empty($filters['entity_id'])) { - $whereConditions[] = "al.entity_id = ?"; - $params[] = $filters['entity_id']; - $paramTypes .= 's'; + $entityId = preg_replace('/[^a-zA-Z0-9_-]/', '', $filters['entity_id']); + if (!empty($entityId)) { + $whereConditions[] = "al.entity_id = ?"; + $params[] = $entityId; + $paramTypes .= 's'; + } } - // Date range filters + // Date range filters - validate format if (!empty($filters['date_from'])) { - $whereConditions[] = "DATE(al.created_at) >= ?"; - $params[] = $filters['date_from']; - $paramTypes .= 's'; + $dateFrom = $this->validateDate($filters['date_from']); + if ($dateFrom !== null) { + $whereConditions[] = "DATE(al.created_at) >= ?"; + $params[] = $dateFrom; + $paramTypes .= 's'; + } } if (!empty($filters['date_to'])) { - $whereConditions[] = "DATE(al.created_at) <= ?"; - $params[] = $filters['date_to']; - $paramTypes .= 's'; + $dateTo = $this->validateDate($filters['date_to']); + if ($dateTo !== null) { + $whereConditions[] = "DATE(al.created_at) <= ?"; + $params[] = $dateTo; + $paramTypes .= 's'; + } } - // IP address filter + // IP address filter - validate format (basic IP pattern) if (!empty($filters['ip_address'])) { - $whereConditions[] = "al.ip_address LIKE ?"; - $params[] = '%' . $filters['ip_address'] . '%'; - $paramTypes .= 's'; + // Allow partial IP matching but sanitize input + $ipAddress = preg_replace('/[^0-9.:a-fA-F]/', '', $filters['ip_address']); + if (!empty($ipAddress) && strlen($ipAddress) <= 45) { // Max IPv6 length + $whereConditions[] = "al.ip_address LIKE ?"; + $params[] = '%' . $ipAddress . '%'; + $paramTypes .= 's'; + } } // Build WHERE clause