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.
This commit is contained in:
Joel Brock
2026-10-30 11:18:10 -07:00
parent c9b610590e
commit 0a19b57a86
@@ -28,17 +28,43 @@
* *
* Returns JSON: { id, name } on success, { error } with 4xx/5xx otherwise. * Returns JSON: { id, name } on success, { error } with 4xx/5xx otherwise.
* *
* Authorization: `access CiviCRM`. AuthX is expected to authenticate the * Authorization: `access CiviCRM`, plus edit permission on the target
* Bearer + Site-Key headers WebForm-mw sends. * 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 { 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() { public function run() {
if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') { if (($_SERVER['REQUEST_METHOD'] ?? 'GET') !== 'POST') {
$this->jsonError('POST required', 405); $this->jsonError('POST required', 405);
return; return;
} }
if (($_SERVER['HTTP_X_REQUESTED_WITH'] ?? '') !== 'XMLHttpRequest') {
$this->jsonError('Missing X-Requested-With header', 403);
return;
}
if (!CRM_Core_Permission::check('access CiviCRM')) { if (!CRM_Core_Permission::check('access CiviCRM')) {
$this->jsonError('Permission denied', 403); $this->jsonError('Permission denied', 403);
return; return;
@@ -69,9 +95,23 @@ class CRM_WebformMw_Page_Upload extends CRM_Core_Page {
$this->jsonError('Invalid entity_id', 400); $this->jsonError('Invalid entity_id', 400);
return; 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'); $name = (string) ($_POST['name'] ?? $upload['name'] ?? 'upload');
$mime = (string) ($_POST['mime_type'] ?? $upload['type'] ?? 'application/octet-stream'); $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 { try {
$result = civicrm_api3('Attachment', 'create', [ $result = civicrm_api3('Attachment', 'create', [