Page MenuHomePhabricator

Fix an error in the PolicyFilter algorithm
ClosedPublic

Authored by epriestley on Dec 5 2013, 10:59 PM.
Tags
None
Referenced Files
F13277090: D7721.diff
Fri, May 31, 8:24 AM
F13265877: D7721.diff
Tue, May 28, 6:52 AM
F13262880: D7721.id17440.diff
Mon, May 27, 4:40 AM
F13249002: D7721.id.diff
Fri, May 24, 5:58 AM
F13246581: D7721.diff
Thu, May 23, 11:41 AM
F13197996: D7721.diff
May 13 2024, 2:50 AM
F13180588: D7721.diff
May 9 2024, 1:15 AM
Unknown Object (File)
May 5 2024, 5:36 AM
Subscribers
Tokens
"Grey Medal" token, awarded by btrahan.

Details

Reviewers
btrahan
Commits
Restricted Diffusion Commit
rPfaaaff0b6f60: Fix an error in the PolicyFilter algorithm
Summary

PhabricatorPolicyFilter has a bug right now where it lets through objects incorrectly if:

  • the query requests two or more policies;
  • the object satisfies at least one of those policies; and
  • policy exceptions are not enabled.

This would be bad, but there's only one call in the codebase which satisfies all of these conditions, in the Maniphest batch editor. And it's moot anyway because edit operations get another policy check slightly later. So there is no policy/security impact from this flaw.

(The next diff relies on this behavior, which is how I caught it.)

Test Plan
  • Added a failing unit test and made it pass.
  • Grepped the codebase for requireCapabilities() and verified that there is no security impact. Basically, 99% of callsites use executeOne(), which throws anyway and moots the filtering.

Diff Detail

Branch
policyfilter
Lint
Lint Passed
Unit
Tests Passed