From 944b257d5df39ba9a7f5c29f271aa8fc27e1ed28 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 31 May 2022 10:55:01 -0700 Subject: [PATCH] Fix a policy issue where permissions were not properly checked when disabling global builtin queries Summary: See . The pathway for disabling global builtin queries is missing a policy check. Add it. Test Plan: - Accessed the "/search/delete/id/.../" URI for a global builtin query as a non-administrator. - Before patch: could improperly disable queries. -After patch: proper policy exception. Differential Revision: https://secure.phabricator.com/D21851 --- .../PhabricatorSearchDeleteController.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/applications/search/controller/PhabricatorSearchDeleteController.php b/src/applications/search/controller/PhabricatorSearchDeleteController.php index 9cbabd3a2f..fe3836017f 100644 --- a/src/applications/search/controller/PhabricatorSearchDeleteController.php +++ b/src/applications/search/controller/PhabricatorSearchDeleteController.php @@ -42,6 +42,19 @@ final class PhabricatorSearchDeleteController } $named_query = $engine->getBuiltinQuery($key); + + // After loading a global query, make sure the viewer actually has + // permission to view and edit it. + + PhabricatorPolicyFilter::requireCapability( + $viewer, + $named_query, + PhabricatorPolicyCapability::CAN_VIEW); + + PhabricatorPolicyFilter::requireCapability( + $viewer, + $named_query, + PhabricatorPolicyCapability::CAN_EDIT); } $builtin = null;