_prod_check_anonymous_rights() false positive on "view uploaded files" permission [#1673276]

If a site has the "view uploaded files" permission enabled (which, in a lot of cases is harmless), the regex in _prod_check_anonymous_rights() triggers, and claims the anonymous users has elevated privileges.

Perhaps this regex should be a little more restrictive (only match the beginnings of permission-strings?), or this one permission-string could be whitelisted?

Comments

Comment #1

malc0mn CreditAttribution: malc0mn commented 6 July 2012 at 22:03

Thanks for the report! The regex is this one:

  preg_match('/(access\sall|add|administer|change|clear|create|delete|edit|revert|save|send\smail|set\svariable|update|upload|PHP|devel)/i, $perms)

Quite silly, but in this case upload matches uploaded as well. When implementing this one I knew we would come across false positives at some point. How about this:

  preg_match('/(access\sall|add|administer|change|clear|create|delete|edit|revert|save|send\smail|set\svariable|update|\bupload\b|PHP|devel)/i, $perms)

for a fix?

Don't want to go slapping word boundary on all of them just yet...

Comment #2

smokris

Athens, Ohio, USA

CreditAttribution: smokris commented 7 July 2012 at 02:40

Oh, the \b regex sequence. I always forget about that one. For resolving the "view uploaded files" problem, that sounds good to me.

However, I expect there to be other false-positives, especially where permissions include user-generated content — I'm thinking about a case where somebody's built a retail store listing with a field_store_address; content_permissions.module would produce a "view field_store_address" permission, which would trigger on the "add" keyword (though anonymous users should legitimately be able to view store addresses).

I think matching all keywords only on word boundaries might actually be a good solution, since the Drupal permissions convention is to separate English words with spaces, so the regex would still trigger correctly whenever a keyword is a standalone word (I'm not thinking of any counterexamples), but avoid false-positives like this issue and the field_store_address example above.