File redirect: per-entity permission check before minting fcs

Security-review follow-up to b65bc6d. The file-redirect route signs an
fcs JWT that Civi's /civicrm/file handler accepts as proof of access.
Any user with the base `access CiviCRM` permission could iterate file
IDs and have us laundering tokens past the entity-level ACLs that would
normally apply (e.g. a staff user without view permission on a given
contact could still pull files attached to that contact).

Tighten it:

- Resolve the file's linked entity_table + entity_id (was: entity_id only).
- Run the entity-type's native permission check before signing the JWT:
    civicrm_activity -> CRM_Activity_BAO_Activity::checkPermission
    civicrm_contact  -> CRM_Contact_BAO_Contact_Permission::allow
  Unknown entity types deny by default — adding a new type requires an
  explicit edit here, so we don't accidentally widen the surface.
- Drop JWT lifetime from a week to 10 minutes. The token is minted at
  click time (the user hits this route fresh on each file click), so
  the long lifetime served no purpose and made each URL a longer-lived
  bearer credential.

Files missing from civicrm_entity_file or pointing at unsupported entity
types now 403 via CRM_Utils_System::permissionDenied() instead of
producing a download URL.
This commit is contained in:
Joel Brock
2026-06-10 10:49:28 -07:00
parent b65bc6d0e0
commit 41467bd4cf
@@ -9,51 +9,51 @@
* site's crypto key, which we have here because we're running inside * site's crypto key, which we have here because we're running inside
* CiviCRM; the Next.js side doesn't. * CiviCRM; the Next.js side doesn't.
* *
* Permission: `access CiviCRM`. The user is already authenticated in the * URL shape: /civicrm/webform-mw/file?id=<fileId>
* parent Civi tab when they click a link in the iframe; their session
* cookie travels with the new-tab navigation.
* *
* URL shape: * Authorization: minting an fcs JWT is effectively "issuing a bearer
* /civicrm/webform-mw/file?id=<fileId>[&eid=<entityId>] * credential for this file" — once issued, /civicrm/file will serve the
* * bytes against any session. To avoid becoming a credential-laundering
* Behavior: * IDOR, we look up the file's linked entity and run the entity-type's
* - Resolve `eid` from civicrm_entity_file if not provided. * native permission check before signing. The base `access CiviCRM`
* - Mint a short-lived JWT with payload {exp, civi.file: <fileId>} * permission only gates reaching this endpoint at all; per-record ACLs
* matching Civi's own /civicrm/file token format. * happen here. Unknown entity types are denied by default.
* - 302-redirect to /civicrm/file?reset=1&id=...&eid=...&fcs=<jwt>.
*/ */
class CRM_WebformMw_Page_File extends CRM_Core_Page { class CRM_WebformMw_Page_File extends CRM_Core_Page {
public function run() { public function run() {
$fileId = (int) CRM_Utils_Request::retrieve('id', 'Positive', $this, TRUE); $fileId = (int) CRM_Utils_Request::retrieve('id', 'Positive', $this, TRUE);
$eid = (int) CRM_Utils_Request::retrieve('eid', 'Positive', $this, FALSE, 0);
// Resolve eid from the entity_file join if the caller didn't supply // Look up the file's linked entity. We need both entity_table and
// one. Any linked entity works for URL-fingerprint purposes; the JWT // entity_id — entity_table drives which permission API to call.
// we mint below is what Civi actually authenticates on. $dao = CRM_Core_DAO::executeQuery(
if (!$eid) { "SELECT entity_table, entity_id FROM civicrm_entity_file WHERE file_id = %1 LIMIT 1",
$dao = CRM_Core_DAO::executeQuery( [1 => [$fileId, 'Positive']]
"SELECT entity_id FROM civicrm_entity_file WHERE file_id = %1 LIMIT 1", );
[1 => [$fileId, 'Positive']] if (!$dao->fetch()) {
); CRM_Utils_System::permissionDenied();
if ($dao->fetch()) { return;
$eid = (int) $dao->entity_id; }
} $entityTable = (string) $dao->entity_table;
$entityId = (int) $dao->entity_id;
if (!$this->canViewEntity($entityTable, $entityId)) {
CRM_Utils_System::permissionDenied();
return;
} }
// Mint the fcs JWT. Payload matches the structure Civi's own file // Mint a short-lived fcs JWT. The token is minted at click time (not
// URL builder emits: {exp, "civi.file": "<id>"}. One-week lifetime — // at report-render time), so 10 minutes covers normal redirect-and-fetch
// these links are typically clicked seconds after the report renders, // latency without making the URL a long-lived bearer credential.
// but the staff report can be left open for a while in a Civi tab.
$payload = [ $payload = [
'exp' => time() + 60 * 60 * 24 * 7, 'exp' => time() + 60 * 10,
'civi.file' => (string) $fileId, 'civi.file' => (string) $fileId,
]; ];
$fcs = \Civi::service('crypto.jwt')->encode($payload); $fcs = \Civi::service('crypto.jwt')->encode($payload);
$url = CRM_Utils_System::url( $url = CRM_Utils_System::url(
'civicrm/file', 'civicrm/file',
"reset=1&id={$fileId}&eid={$eid}&fcs=" . urlencode($fcs), "reset=1&id={$fileId}&eid={$entityId}&fcs=" . urlencode($fcs),
FALSE, FALSE,
NULL, NULL,
FALSE, FALSE,
@@ -62,4 +62,31 @@ class CRM_WebformMw_Page_File extends CRM_Core_Page {
CRM_Utils_System::redirect($url); CRM_Utils_System::redirect($url);
} }
/**
* Check whether the current Civi user can view the entity a file is
* linked to. Restricted to the entity types the WebForm-mw staff report
* actually surfaces (activity custom-field files and contact custom-field
* files); everything else denies. New entity types should be added here
* deliberately so we don't accidentally widen the surface.
*/
private function canViewEntity(string $entityTable, int $entityId): bool {
if ($entityId <= 0) {
return FALSE;
}
switch ($entityTable) {
case 'civicrm_activity':
return CRM_Activity_BAO_Activity::checkPermission(
$entityId,
CRM_Core_Permission::VIEW
);
case 'civicrm_contact':
return CRM_Contact_BAO_Contact_Permission::allow(
$entityId,
CRM_Core_Permission::VIEW
);
default:
return FALSE;
}
}
} }