From 8fd50ea580e88541fed884b5a5da49629d66a4b2 Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Thu, 18 Jun 2026 09:48:23 -0500 Subject: [PATCH] =?UTF-8?q?fix(security+data):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20XSS,=20header=20injection,=20ACL,=20status=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Remove SVG from allowed upload extensions (stored XSS vector) 2. Sanitize Content-Disposition filename (header injection prevention) 3. Add ACL check to rateTicket() (missing authorization) 4. Use server-side MIME detection instead of client-supplied type 5. Sync status_id/priority_id in AutomationEngine and API controller to keep ENUM and lookup-table columns consistent 6. Align auto-bump.yml to use mokoplatform (no hyphen) matching pre-release.yml convention --- .mokogitea/workflows/auto-bump.yml | 18 ++--- .../src/Controller/DisplayController.php | 4 +- .../admin/src/Service/AttachmentService.php | 4 +- .../admin/src/Service/AutomationEngine.php | 68 +++++++++++++++---- .../api/src/Controller/TicketsController.php | 25 +++++-- 5 files changed, 88 insertions(+), 31 deletions(-) diff --git a/.mokogitea/workflows/auto-bump.yml b/.mokogitea/workflows/auto-bump.yml index 34953b1d..022148e0 100644 --- a/.mokogitea/workflows/auto-bump.yml +++ b/.mokogitea/workflows/auto-bump.yml @@ -4,8 +4,8 @@ # # FILE INFORMATION # DEFGROUP: Gitea.Workflow -# INGROUP: moko-platform.Release -# REPO: https://git.mokoconsulting.tech/MokoConsulting/moko-platform +# INGROUP: mokoplatform.Release +# REPO: https://git.mokoconsulting.tech/MokoConsulting/mokoplatform # PATH: /.mokogitea/workflows/auto-bump.yml # VERSION: 09.02.00 # BRIEF: Auto patch-bump version on every push to dev (skips merge commits) @@ -43,19 +43,19 @@ jobs: token: ${{ secrets.MOKOGITEA_TOKEN }} fetch-depth: 1 - - name: Setup moko-platform tools + - name: Setup mokoplatform tools run: | if ! command -v composer &> /dev/null; then sudo apt-get update -qq && sudo apt-get install -y -qq php-cli php-mbstring php-xml php-zip php-curl composer >/dev/null 2>&1 fi - if [ -d "/opt/moko-platform/cli" ]; then - echo "MOKO_CLI=/opt/moko-platform/cli" >> "$GITHUB_ENV" + if [ -d "/opt/mokoplatform/cli" ]; then + echo "MOKO_CLI=/opt/mokoplatform/cli" >> "$GITHUB_ENV" else git clone --depth 1 --branch main --quiet \ - "https://x-access-token:${{ secrets.MOKOGITEA_TOKEN }}@git.mokoconsulting.tech/MokoConsulting/moko-platform.git" \ - /tmp/moko-platform-api - cd /tmp/moko-platform-api && composer install --no-dev --no-interaction --quiet - echo "MOKO_CLI=/tmp/moko-platform-api/cli" >> "$GITHUB_ENV" + "https://x-access-token:${{ secrets.MOKOGITEA_TOKEN }}@git.mokoconsulting.tech/MokoConsulting/mokoplatform.git" \ + /tmp/mokoplatform-api + cd /tmp/mokoplatform-api && composer install --no-dev --no-interaction --quiet + echo "MOKO_CLI=/tmp/mokoplatform-api/cli" >> "$GITHUB_ENV" fi - name: Bump version diff --git a/source/packages/com_mokosuiteclient/admin/src/Controller/DisplayController.php b/source/packages/com_mokosuiteclient/admin/src/Controller/DisplayController.php index 0cbf0ad6..a0554548 100644 --- a/source/packages/com_mokosuiteclient/admin/src/Controller/DisplayController.php +++ b/source/packages/com_mokosuiteclient/admin/src/Controller/DisplayController.php @@ -685,7 +685,8 @@ class DisplayController extends BaseController if (!file_exists($path)) { throw new \RuntimeException('File not found', 404); } $app = Factory::getApplication(); $app->setHeader('Content-Type', $att->mimetype ?: 'application/octet-stream'); - $app->setHeader('Content-Disposition', 'attachment; filename="' . $att->filename . '"'); + $safeName = str_replace(['"', "\r", "\n"], '', $att->filename); + $app->setHeader('Content-Disposition', 'attachment; filename="' . $safeName . '"'); $app->setHeader('Content-Length', (string) filesize($path)); $app->sendHeaders(); readfile($path); @@ -704,6 +705,7 @@ class DisplayController extends BaseController public function rateTicket() { Session::checkToken() or die(Text::_('JINVALID_TOKEN')); + if (!$this->checkAcl('mokosuiteclient.tickets')) { $this->jsonForbidden(); } $input = Factory::getApplication()->getInput(); $ticketId = $input->getInt('ticket_id', 0); $rating = $input->getInt('rating', 0); diff --git a/source/packages/com_mokosuiteclient/admin/src/Service/AttachmentService.php b/source/packages/com_mokosuiteclient/admin/src/Service/AttachmentService.php index 2418f894..1ade4802 100644 --- a/source/packages/com_mokosuiteclient/admin/src/Service/AttachmentService.php +++ b/source/packages/com_mokosuiteclient/admin/src/Service/AttachmentService.php @@ -20,7 +20,7 @@ class AttachmentService private const STORAGE_DIR = JPATH_ROOT . '/media/com_mokosuiteclient/attachments'; private const ALLOWED_EXTENSIONS = [ - 'jpg', 'jpeg', 'png', 'gif', 'webp', 'svg', + 'jpg', 'jpeg', 'png', 'gif', 'webp', 'pdf', 'doc', 'docx', 'xls', 'xlsx', 'csv', 'txt', 'rtf', 'zip', 'gz', 'tar', ]; @@ -95,7 +95,7 @@ class AttachmentService 'filename' => $originalName, 'filepath' => $ticketId . '/' . $storedName, 'filesize' => $files['size'][$i], - 'mimetype' => $files['type'][$i], + 'mimetype' => mime_content_type($destPath) ?: 'application/octet-stream', 'uploaded_by' => $userId, 'created' => Factory::getDate()->toSql(), ]; diff --git a/source/packages/com_mokosuiteclient/admin/src/Service/AutomationEngine.php b/source/packages/com_mokosuiteclient/admin/src/Service/AutomationEngine.php index 4fba20f9..52b30a9f 100644 --- a/source/packages/com_mokosuiteclient/admin/src/Service/AutomationEngine.php +++ b/source/packages/com_mokosuiteclient/admin/src/Service/AutomationEngine.php @@ -120,13 +120,19 @@ class AutomationEngine { case 'set_status': if ($ticketId) { - $db->setQuery("UPDATE {$db->quoteName('#__mokosuiteclient_tickets')} SET status = {$db->quote($value)}, modified = {$db->quote(Factory::getDate()->toSql())} WHERE id = {$ticketId}")->execute(); + $statusId = self::resolveStatusId($db, $value); + $sets = "status = {$db->quote($value)}, modified = {$db->quote(Factory::getDate()->toSql())}"; + if ($statusId) { $sets .= ", status_id = {$statusId}"; } + $db->setQuery("UPDATE {$db->quoteName('#__mokosuiteclient_tickets')} SET {$sets} WHERE id = {$ticketId}")->execute(); } break; case 'set_priority': if ($ticketId) { - $db->setQuery("UPDATE {$db->quoteName('#__mokosuiteclient_tickets')} SET priority = {$db->quote($value)}, modified = {$db->quote(Factory::getDate()->toSql())} WHERE id = {$ticketId}")->execute(); + $priorityId = self::resolvePriorityId($db, $value); + $sets = "priority = {$db->quote($value)}, modified = {$db->quote(Factory::getDate()->toSql())}"; + if ($priorityId) { $sets .= ", priority_id = {$priorityId}"; } + $db->setQuery("UPDATE {$db->quoteName('#__mokosuiteclient_tickets')} SET {$sets} WHERE id = {$ticketId}")->execute(); } break; @@ -167,7 +173,10 @@ class AutomationEngine case 'close': if ($ticketId) { - $db->setQuery("UPDATE {$db->quoteName('#__mokosuiteclient_tickets')} SET status = 'closed', closed = {$db->quote(Factory::getDate()->toSql())}, modified = {$db->quote(Factory::getDate()->toSql())} WHERE id = {$ticketId}")->execute(); + $closedId = self::resolveClosedStatusId($db); + $sets = "status = 'closed', closed = {$db->quote(Factory::getDate()->toSql())}, modified = {$db->quote(Factory::getDate()->toSql())}"; + if ($closedId) { $sets .= ", status_id = {$closedId}"; } + $db->setQuery("UPDATE {$db->quoteName('#__mokosuiteclient_tickets')} SET {$sets} WHERE id = {$ticketId}")->execute(); } break; @@ -186,6 +195,30 @@ class AutomationEngine /** * Create a ticket from automation (with behavior: append/always_new/skip_if_open). */ + private static function resolveStatusId($db, string $alias): int + { + return (int) $db->setQuery( + $db->getQuery(true)->select('id')->from('#__mokosuiteclient_ticket_statuses') + ->where($db->quoteName('alias') . ' = ' . $db->quote($alias)), 0, 1 + )->loadResult(); + } + + private static function resolvePriorityId($db, string $alias): int + { + return (int) $db->setQuery( + $db->getQuery(true)->select('id')->from('#__mokosuiteclient_ticket_priorities') + ->where($db->quoteName('alias') . ' = ' . $db->quote($alias)), 0, 1 + )->loadResult(); + } + + private static function resolveClosedStatusId($db): int + { + return (int) $db->setQuery( + $db->getQuery(true)->select('id')->from('#__mokosuiteclient_ticket_statuses') + ->where($db->quoteName('is_closed') . ' = 1'), 0, 1 + )->loadResult(); + } + private static function createTicketFromAutomation(object $rule, array $context, string $subject): void { $db = Factory::getDbo(); @@ -195,12 +228,13 @@ class AutomationEngine if ($behavior !== 'always_new' && $userId > 0) { - // Check for existing open ticket + // Check for existing open ticket (check both status ENUM and status_id) $query = $db->getQuery(true) - ->select('id') - ->from('#__mokosuiteclient_tickets') - ->where('created_by = ' . $userId) - ->where("status NOT IN ('closed', 'resolved')"); + ->select('t.id') + ->from($db->quoteName('#__mokosuiteclient_tickets', 't')) + ->join('LEFT', $db->quoteName('#__mokosuiteclient_ticket_statuses', 's') . ' ON t.status_id = s.id') + ->where('t.created_by = ' . $userId) + ->where("(s.id IS NULL AND t.status NOT IN ('closed', 'resolved')) OR (s.id IS NOT NULL AND s.is_closed = 0)"); if ($catId > 0) { $query->where('category_id = ' . $catId); @@ -227,14 +261,18 @@ class AutomationEngine } // Create new ticket + $openStatusId = self::resolveStatusId($db, 'open') ?: null; + $normalPriorityId = self::resolvePriorityId($db, $context['priority'] ?? 'normal') ?: null; $ticket = (object) [ - 'subject' => $subject ?: 'Automation: ' . ($rule->title ?? ''), - 'body' => $context['body'] ?? '', - 'status' => 'open', - 'priority' => $context['priority'] ?? 'normal', - 'category_id' => $catId ?: null, - 'created_by' => $userId, - 'created' => Factory::getDate()->toSql(), + 'subject' => $subject ?: 'Automation: ' . ($rule->title ?? ''), + 'body' => $context['body'] ?? '', + 'status' => 'open', + 'status_id' => $openStatusId, + 'priority' => $context['priority'] ?? 'normal', + 'priority_id' => $normalPriorityId, + 'category_id' => $catId ?: null, + 'created_by' => $userId, + 'created' => Factory::getDate()->toSql(), ]; $db->insertObject('#__mokosuiteclient_tickets', $ticket, 'id'); } diff --git a/source/packages/com_mokosuiteclient/api/src/Controller/TicketsController.php b/source/packages/com_mokosuiteclient/api/src/Controller/TicketsController.php index 754c1b84..fdbfb1ad 100644 --- a/source/packages/com_mokosuiteclient/api/src/Controller/TicketsController.php +++ b/source/packages/com_mokosuiteclient/api/src/Controller/TicketsController.php @@ -144,13 +144,30 @@ class TicketsController extends BaseController return; } + $statusId = $input->getInt('status_id', 0) ?: null; + $priorityId = $input->getInt('priority_id', 0) ?: null; + $status = $input->getString('status', 'open'); + $priority = $input->getString('priority', 'normal'); + + // Resolve status_id from alias if not provided + if (!$statusId && $status) { + $q = $db->getQuery(true)->select('id')->from('#__mokosuiteclient_ticket_statuses') + ->where($db->quoteName('alias') . ' = ' . $db->quote($status)); + $statusId = (int) $db->setQuery($q, 0, 1)->loadResult() ?: null; + } + if (!$priorityId && $priority) { + $q = $db->getQuery(true)->select('id')->from('#__mokosuiteclient_ticket_priorities') + ->where($db->quoteName('alias') . ' = ' . $db->quote($priority)); + $priorityId = (int) $db->setQuery($q, 0, 1)->loadResult() ?: null; + } + $ticket = (object) [ 'subject' => $subject, 'body' => $body, - 'status' => 'open', - 'status_id' => $input->getInt('status_id', 0) ?: null, - 'priority' => $input->getString('priority', 'normal'), - 'priority_id' => $input->getInt('priority_id', 0) ?: null, + 'status' => $status, + 'status_id' => $statusId, + 'priority' => $priority, + 'priority_id' => $priorityId, 'category_id' => $input->getInt('category_id', 0) ?: null, 'created_by' => (int) Factory::getUser()->id, 'assigned_to' => $input->getInt('assigned_to', 0) ?: null,