Contrib.social

(int) typecast in User::hasPermission() is dangerous

Open on Drupal.org →
Created on 23 November 2024, 29 days ago

This issue was discussed by the Drupal Security Team, and their decision was that this can be solved in a public issue.

Problem/Motivation

@samuel.mortenson noticed this:

User::hasPermission() starts with:

 public function hasPermission($permission) {
    // User #1 has all privileges.
    if ((int) $this->id() === 1) {
      return TRUE;
    }

Note the typecast. If the user ID is some unexpected non-integer value like TRUE or '1e0' or '1. Add a typecast 2. ??? 3. Profit', this could sometimes result in the user being granted all permissions.

The typecast does not exist in Drupal 7; it was introduced in #1966334-54: Convert user_access to User::hasPermission() → without any specific explanation.

Proposed resolution

Get rid of the typecast.

Maybe there are cases where the user ID is '1' as a string from a form submission, so for BC we could check for either '1' or 1.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

8.9 ⚰️

Component

user.module

Created by

🇪🇪Estonia ram4nd Tallinn

Live updates comments and jobs are added and updated live.
  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

Sign in to follow issues

Merge Requests

Comments & Activities

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024