From 698ada2470b18ae0b6ec579614b56b1bee395eba Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 9 May 2022 15:00:10 -0700 Subject: [PATCH] Correct overbroad automatic capability grant of global settings objects Summary: Ref T13679. In D16983, global settings objects were given an exception to let logged-out users see them, even on installs with no "public" user role. This exception is too broad and grants everyone all capabilities, not just "CAN_VIEW". In particular, it incorrectly grants "CAN_EDIT", so any user can edit global settings defaults. Restrict this grant to "CAN_VIEW". Test Plan: - As a non-administrator, tried to edit global settings. - Before: could. - After: could not. Maniphest Tasks: T13679 Differential Revision: https://secure.phabricator.com/D21811 --- .../storage/PhabricatorUserPreferences.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php index 0af6b4553c..6cceff57de 100644 --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -219,11 +219,15 @@ final class PhabricatorUserPreferences } } - switch ($this->getBuiltinKey()) { - case self::BUILTIN_GLOBAL_DEFAULT: - // NOTE: Without this policy exception, the logged-out viewer can not - // see global preferences. - return true; + $builtin_key = $this->getBuiltinKey(); + + $is_global = ($builtin_key === self::BUILTIN_GLOBAL_DEFAULT); + $is_view = ($capability === PhabricatorPolicyCapability::CAN_VIEW); + + if ($is_global && $is_view) { + // NOTE: Without this policy exception, the logged-out viewer can not + // see global preferences. + return true; } return false;