Problem/Motivation

</code>PrivatemsgController::blockUser
(src/Controller/PrivatemsgController.php) is the controller behind
privatemsg.block_user (/messages/block/{user}). It toggles the block
relationship for the current user and the target, but performs no
canBeBlocked() check and no self-block check:

$isBlocked = $this->privateMsgService->isUserBlocked($this->currentUser()->id(), $user_id);
...
if ($isBlocked) {
  $this->privateMsgService->unblockUser($user_id);
  ...
}
$this->privateMsgService->blockUser($user_id);

The unblockable_roles check exists only in two places that do not protect this endpoint:

1. PrivatemsgBlockUserForm::submitForm checks canBeBlocked() before blocking
(form path only).
2. PrivatemsgHooks::preprocessPrivatemsgMessage hides the block link for
unblockable users (UI only).

A direct request to /messages/block/{uid} by any user with the
privatemsg block users permission therefore can:

  • block a moderator / any user in an unblockable_roles role — the whole point of the setting is that those accounts must not be blockable; and
  • block themselves — the toggle writes a pm_block_user row where who = blocked = current user, after which isUserBlocked() always returns true for that pair and the user can no longer be messaged.

This is reachable even though the UI hides the link, because the route is a
plain controller route with no argument validation.

Steps to reproduce

1. Configure privatemsg.settings with unblockable_roles: [llamalovers] and
give user B the llamalovers role (e.g. a moderator).
2. Log in as user A with privatemsg write messages + privatemsg block users.
3. Visit /messages/block/{B} directly (or trigger it from crafted markup
4. User B is now blocked even though their role is unblockable; user A has also never been prevented from blocking themselves via /messages/block/{A}.

Proposed resolution

In PrivatemsgController::blockUser, reject the request (400) when the action is a block (i.e. the pair is not already blocked, so the toggle would call blockUser()) and either condition holds:

if (!$isBlocked && ((int) $user_id === (int) $this->currentUser()->id() || !$this->privateMsgService->canBeBlocked($user_id))) {
  return $incorrect_response;
}

The !$isBlocked guard preserves the legitimate unblock direction of the toggle (an account that was blocked before gaining an unblockable role can still be unblocked).

Unblockable-role and self-block cases are rejected; the unblockable_roles configuration is honored on every code path, not just the form.

Remaining tasks

Patch with tests is forthcoming.

Issue fork privatemsg-3617048

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

lisa.rae created an issue. See original summary.

lisa.rae’s picture

Issue summary: View changes

lisa.rae’s picture

Status: Active » Needs review

ivnish changed the visibility of the branch 3617048-block-endpoint-bypasses to hidden.

  • ivnish committed 9546be65 on 2.0.x authored by lisa.rae
    fix: #3617048 Block endpoint bypasses the `unblockable_roles` setting (...
ivnish’s picture

Status: Needs review » Fixed

Nice catch! Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.