Do not allow existing or reserved paths as aliases

Created on 21 February 2007, over 17 years ago
Updated 26 March 2024, 3 months ago

Problem/Motivation

In Drupal 8 and earlier, it is possible for a user to create a path alias that clobbers an essential core route such as "admin" or "user/login". For example, a malicious user with permission to create content using an unfiltered text format and to "Create and edit URL aliases" could override the "user/login" path with a node crafted to look like it but with an HTML form that posted the submission to another URL that harvested the login credentials. Of course less serious abuses can be easily imagined, such as merely rendering certain admin pages inaccessible.

Proposed resolution

  • Adds a setting for restricted path alias patterns
  • Adds a validation constraint to enforce it
  • Adds a permission to bypass it that admins can use

Remaining tasks

  • Decide if this is a bug or a task, the presence of new API makes me think its a task
  • Reroll the patch at #67 as an MR
  • Document the new setting in default.settings.php
  • Add test coverage
  • Review
  • Commit

User interface changes

None

API changes

New validation constraint and permission

Release notes snippet

Todo

πŸ“Œ Task
Status

Needs work

Version

11.0 πŸ”₯

Component
PathΒ  β†’

Last updated 6 days ago

  • Maintained by
  • πŸ‡¬πŸ‡§United Kingdom @catch
Created by

πŸ‡ΊπŸ‡ΈUnited States JHeffner

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

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¦πŸ‡ΉAustria tgoeg

    Just lost my (lengthy) comment because of a Firefox crash, I'll keep it shorter, ask for details if needed, please!

    I do think this should be handled as a security issue.
    With a respective setup for example, even unauthenticated users might take over core paths on sites where some comment functionality or similar might create aliases (e.g. Path Auto setup to make human readable aliases of comments' URLs by using the title or first few words as pattern (this is a common feature you see on the Internet). I also don't see how an editor can create a legit looking /user/login site without this "feature".

    Ran into this with a site using permissions by term. A node with alias /batch had been created. Afterwards, a user was created, permissions by term then drops all privileges and rebuilds them. In batches :). Guess what happened. No rebuilt permissions as the alias caught the request to /batch&id=132, site completely broken.

    The security issue alone should warrant a "critical" priority I think, even more now that I see this is postponed to 11.x which means this issue that's already 6 years old will have to wait a few more for a fix.

    As stated above, we need to come to a consensus how this should be solved before updating the IS. Reading through this whole thread shows a few different use-cases I try to cover.
    Here's my suggestion:

    1. Provide a separate permission for a manual override, as suggested in https://www.drupal.org/project/drupal/issues/121362#comment-7437458 πŸ› Do not allow existing or reserved paths as aliases Needs work , e.g. "Override core URL aliases", though I'd call it "Override existing, whitelisted URLs (created by core/modules) with aliases". Add a link to the config page.
    2. Provide another permission, "Override any existing URL with an alias", meant only for admins. This way, a granular configuration of what editors may override can be performed without restricting admins that may need to override even more.
    3. Build a list of found core/module(/file, see below) paths, updated probably in cron runs and module/core installation steps. I would not hardcode this, as it can change quickly.
    4. Provide a config page as can be seen in this mockup. Shamelessly copied from the config_split module; there might also be code there that can be reused for the list/form:
      1. String-based whitelist for aliases anticipated but not existing yet. Or for specific ones like /node/123 (which should not be part of the automatic list)
      2. Blacklist might not be necessary, as everything not on the whitelist should be blacklisted, anyway. Might however be practical to reduce whitelist-writing efforts, e.g. whitelist /node/* but blacklist /node/1. This would be tedious with whitelist only.
    5. Show a warning when saving a page that overrides an alias.
    6. Show an error when trying to save a page and permissions/white-/blacklists do not allow overriding as in https://www.drupal.org/project/drupal/issues/121362#comment-13013100 πŸ› Do not allow existing or reserved paths as aliases Needs work
    7. The whitelist/blacklist should also apply to modules like e.g. pathauto. If a module tries to assign an alias that is not allowed, at least log that to the DB log. Better yet, show it in the status page and the top of the admin interface, so people notice something is going wrong. Unsure whether this should be part of core's path functionality or every module itself. Sounds more like the former, as it will catch all errors.
      This, together with the dynamically built list, also solves the problem of existing white-/blacklists and URLs that get added afterwards.
    8. Unsure how to handle aliases that resolve to filesystem paths. Those should be forbidden by default. Probably also determine them in the list-building process and explicitly add them to the blacklist. Or make a separate list for them.

    This should implicitly answer all the questions in https://www.drupal.org/project/drupal/issues/121362#comment-14103203 πŸ› Do not allow existing or reserved paths as aliases Needs work with "Yes, configurable as outlined in the mockup".

    A completely different strategy might be to explicitly trigger a request to the alias that shall be added and determine whether it is free by the answer received.

  • πŸ‡¬πŸ‡§United Kingdom catch

    @tgoeg "I do think this should be handled as a security issue."

    Handling this as a security issue would mean unpublishing this issue, opening a new issue in the private security tracker, and then any solution having to go through the core security release process. Given this issue has been open for 16 years in the public queue that is not going to stop anyone finding out about it. It's also very hard/impossible to do security releases with significant behaviour changes like a new configuration page, which is something that would normally only be introduced in a minor release, not to (at this time) 9.4.x

    I think there's an argument this allows users in certain configurations to DOS a site though, so agreed with making it a critical bug.

  • πŸ‡¦πŸ‡ΉAustria tgoeg

    The 'net does not forget, as they say.

    I totally agree with you. I did not want to change the issue administration-wise, but reflect that there definitely is a security aspect to it, which should not be taken lightheartedly (which I felt was the case, reading the above comments).
    If a critical priority reduces the time to fix this, this should be enough. Knowing that 9.x and even 10.x will remain without a fix however feels pretty uncomfortable. I think we should at least strive for a patch that applies to 10.x if it really is too late for any changes in this major series..

  • πŸ‡¬πŸ‡§United Kingdom catch

    The change could land in a minor release (i.e. the next one would be 10.2.x now that 10.1.0 is at rc stage), it just won't land in a patch release if it's adding new configuration and a UI for it.

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    This is now the oldest open bug in core.

    Taking a look at it for Bug Smash Initiative.

    What is being added here looks like a new API - so perhaps this should be a task.

    In that case we would call it 'Add a restricted patterns setting to limit what can be used as path aliases'

    The current patch does the following:

    * Adds a setting for restricted path alias patterns
    * Adds a validation constraint to enforce it
    * Adds a permission to bypass it that admins can use

    I think this is a reasonable approach to getting something in - this issue has been open for 17 years.

    The next steps are:
    * Reroll the patch as an MR
    * Add test coverage
    * Document the setting in the default.settings.php file.

    Does anyone feel strongly about keeping this as a bug?

  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    kim.pepper β†’ made their first commit to this issue’s fork.

  • Pipeline finished with Failed
    3 months ago
    Total: 492s
    #129097
  • πŸ‡¦πŸ‡ΊAustralia pameeela

    I came here to see whether there was any discussion of bug vs task :)

    Oddly enough, just last week we had a use case to redirect /user/register because they are about to launch a new site and wanted to pause registrations. A redirect to a content page explaining the situation was by far the easiest way to achieve this, because it was linked from many places across the site.

    So this combined with the fact that it generally feels like an add-on leads me to task.

  • πŸ‡¬πŸ‡§United Kingdom catch

    Yeah I bumped this to critical in #75, because I think the example given that certain combination of permissions are able to do worse things due to this issue than they otherwise would be is a good one, but I also think it's a task. There's no functional bug as such, this was intended behaviour when it was added, it's just that this issue wants to change that behaviour to make things more locked down.

Production build 0.69.0 2024