Problem/Motivation

This was originally submitted as a security issue but was approved by the security team for moving to the public queue.

Core uses gzuncompress() against the include/exclude and ajax_page_state query parameters, but doesn't pass the second $max_length parameter, this means it's possible to construct a very long URL and exhaust memory.

https://www.php.net/manual/en/function.gzuncompress.php

We can set a sensible maximum that is at the upper bounds of what we'd expect a browser to support or Drupal to ever possibly need, and that should be fine.

Security issues with be a 403 for the majority of people, but including references for people with access:

https://git.drupalcode.org/security/185187-drupal-security/-/work_items/1
https://git.drupalcode.org/security/185308-drupal-security/-/work_items/1

Steps to reproduce

Proposed resolution

Set the length to 3000 characters. For comparison this asset URL from Umami is approximately 500 characters in its entirety:

https://drupal-dev.ddev.site/sites/default/files/css/css_jrzzmtxd42TQDdxeoyfnAZXSeX3_xjqwZrdWCI_jiKk.css?delta=0&language=en-gb&theme=umami&include=eJx9kFFuAzEIRC9kr8-EbbJFwbAC77bp6esmaaMocn6QeDMawRSVjl99B07V9g14Kf8kMsnZQ1HDVLRtKijdF4GDVuikEmNDd1jxraercgaLee9d5cW6N2gUYwYRtKlqIJVknekFrM60Sl4YqM3TT6odx4Gs5TzzfCDUeYINNbah3Qz3Kp_aEbUGTN8YHuUkGl2bDOuDBb94x5YyOIZrfBrnu1-WVyJa_8jKmoHvi2GhDWNRZiy_oR4Owk9P17k0rTvjEzL08ZLTgXE1qj_aJM34

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3609975

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

catch created an issue. See original summary.

catch’s picture

Title: gzuncompress() calls (aggregation mostly) should set the max_length parameter » UrlHelper::uncompressQueryParameter() should set a gzuncompress() character limit
catch’s picture

Status: Active » Needs review
catch’s picture

Issue summary: View changes
catch’s picture

I think I read the docs wrong, the limit is for the length of the uncompressed data, I think we can safely set that to 10kb since that would be a lot