From 0a19b57a8676d7ae5db5e9ac6743bede2678d7f1 Mon Sep 17 00:00:00 2001
From: Joel Brock <joelbrock@gmail.com>
Date: Fri, 30 Oct 2026 11:18:10 -0700
Subject: [PATCH] Harden the extension upload endpoint (CSRF, record ACL, mime
 allowlist)

civicrm/webform-mw/upload accepted any session-authenticated POST, called
civicrm_api3('Attachment','create') (which skips permission checks from
PHP) against any contact/activity id, and stored any client mime type.

- Require X-Requested-With: XMLHttpRequest (already sent by the app), so
  cross-site form posts can't ride a logged-in staff session.
- Require EDIT permission on the target contact/activity.
- Enforce the same mime allowlist as the app's /api/upload route.
---
 .../webform-mw/CRM/WebformMw/Page/Upload.php  | 44 ++++++++++++++++++-
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php b/civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php
index 14b0e30..673be86 100644
--- a/civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php
+++ b/civi-extension/webform-mw/CRM/WebformMw/Page/Upload.php
@@ -28,17 +28,43 @@
  *
  * Returns JSON: { id, name } on success, { error } with 4xx/5xx otherwise.
  *
- * Authorization: `access CiviCRM`. AuthX is expected to authenticate the
- * Bearer + Site-Key headers WebForm-mw sends.
+ * Authorization: `access CiviCRM`, plus edit permission on the target
+ * contact/activity. AuthX is expected to authenticate the Bearer +
+ * Site-Key headers WebForm-mw sends.
+ *
+ * CSRF: browsers can't attach a custom header to a cross-site form post
+ * without a CORS preflight, which Civi doesn't grant. Requiring
+ * `X-Requested-With: XMLHttpRequest` (which WebForm-mw already sends)
+ * stops a malicious page from riding a logged-in staff session.
  */
 class CRM_WebformMw_Page_Upload extends CRM_Core_Page {
 
+  /**
+   * Mirrors ALLOWED_MIME in the app's /api/upload route.
+   */
+  const ALLOWED_MIME = [
+    'application/pdf',
+    'application/msword',
+    'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
+    'application/vnd.ms-excel',
+    'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
+    'image/jpeg',
+    'image/png',
+    'image/gif',
+    'image/webp',
+  ];
+
   public function run() {
     if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') {
       $this->jsonError('POST required', 405);
       return;
     }
 
+    if (($_SERVER['HTTP_X_REQUESTED_WITH'] ?? '') !== 'XMLHttpRequest') {
+      $this->jsonError('Missing X-Requested-With header', 403);
+      return;
+    }
+
     if (!CRM_Core_Permission::check('access CiviCRM')) {
       $this->jsonError('Permission denied', 403);
       return;
@@ -69,9 +95,23 @@ class CRM_WebformMw_Page_Upload extends CRM_Core_Page {
       $this->jsonError('Invalid entity_id', 400);
       return;
     }
+    // civicrm_api3() from PHP skips permission checks, so check the target
+    // record here — otherwise any Civi user could attach files to any
+    // contact or activity.
+    $canEdit = $entityTable === 'civicrm_contact'
+      ? CRM_Contact_BAO_Contact_Permission::allow($entityId, CRM_Core_Permission::EDIT)
+      : CRM_Activity_BAO_Activity::checkPermission($entityId, CRM_Core_Permission::EDIT);
+    if (!$canEdit) {
+      $this->jsonError('Permission denied', 403);
+      return;
+    }
 
     $name = (string) ($_POST['name'] ?? $upload['name'] ?? 'upload');
     $mime = (string) ($_POST['mime_type'] ?? $upload['type'] ?? 'application/octet-stream');
+    if (!in_array($mime, self::ALLOWED_MIME, TRUE)) {
+      $this->jsonError('File type not allowed', 415);
+      return;
+    }
 
     try {
       $result = civicrm_api3('Attachment', 'create', [