drupal_render_cid_parts($granularity) in common.inc includes the following comment:
'PER_ROLE' and 'PER_USER' are mutually exclusive. 'PER_USER' can be a
resource drag for sites with many users, so when a module is being
equivocal, we favor the less expensive 'PER_ROLE' pattern.
If I have included PER_USER in $granularity (even if I have also specified other flags) then that means I don't want two users seeing each others content, and must accept any performance hit that implies. Instead, Drupal is currently silently assuming there's a bug in my module and instead of throwing an error it silently chooses an option that could reveal confidential data to less privileged users! This is therefore a security vulnerability, even if it isn't intentionally exploitable.
While I agree it is pointless to set both PER_ROLE and PER_USER, they are not actually mutually exclusive. I can't think of any reason you can't include both the role and user in the cache key, and the only reasoning given seems to be to avoid the performance implications of PER_USER.
I was initially going to suggest that if both PER_USER and PER_ROLE were set then we could cache based on the user only, but then I realised there is an edge case where users may not immediately gain/lose access to cached content when they are given a new role. Instead I think we must remove the else and the comment and will attach a patch that does this.
(I came across this bug when testing a patch for another issue, where I assumed that
if ($granularity & DRUPAL_CACHE_PER_USER) then I was safe to include user-specific information in the block. This is currently not the case.)
FAILED: [[SimpleTest]]: [MySQL] 41,133 pass(es), 1 fail(s), and 0 exception(s). View
PASSED: [[SimpleTest]]: [MySQL] 58,047 pass(es). View
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1966346-cache-per-user-1.patch. Unable to apply patch. See the log in the details link for more information. View