Commit fc039e59 authored by catch's avatar catch
Browse files

fix: #3603762 HTML injection in ajax_page_state GET parameter 

By: prudloff
By: longwave
By: smustgrave
(cherry picked from commit 2fdf57a0)
parent 8054d99e

core/lib/Drupal/Core/Asset/CssCollectionOptimizerLazy.php

+1 −1
Original line number Diff line number Diff line
@@ -108,7 +108,7 @@ public function optimize(array $css_assets, array $libraries) { 
      'include' => UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($libraries))),

    ];

    $ajax_page_state = $this->requestStack->getCurrentRequest()->attributes->get('ajax_page_state');

    $already_loaded = isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : [];

    $already_loaded = isset($ajax_page_state['libraries']) ? explode(',', $ajax_page_state['libraries']) : [];

    if ($already_loaded) {

      $query_args['exclude'] = UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($already_loaded)));

    }

core/lib/Drupal/Core/Asset/JsCollectionOptimizerLazy.php

+1 −1
Original line number Diff line number Diff line
@@ -116,7 +116,7 @@ public function optimize(array $js_assets, array $libraries) { 
        'include' => UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($libraries))),

      ];

      $ajax_page_state = $this->requestStack->getCurrentRequest()->attributes->get('ajax_page_state');

      $already_loaded = isset($ajax_page_state) ? explode(',', $ajax_page_state['libraries']) : [];

      $already_loaded = isset($ajax_page_state['libraries']) ? explode(',', $ajax_page_state['libraries']) : [];

      if ($already_loaded) {

        $query_args['exclude'] = UrlHelper::compressQueryParameter(implode(',', $this->dependencyResolver->getMinimalRepresentativeSubset($already_loaded)));

      }

core/lib/Drupal/Core/StackMiddleware/AjaxPageState.php

+22 −1
Original line number Diff line number Diff line
@@ -29,14 +29,24 @@ public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = TR 
      $request_ajax_page_state = [];

      if ($request->request->has('ajax_page_state')) {

        $request_ajax_page_state = $this->parseAjaxPageState($request->request->all('ajax_page_state'));

        if (!empty($request_ajax_page_state)) {

          $request->request->set('ajax_page_state', $request_ajax_page_state);

        }

        else {

          $request->request->remove('ajax_page_state');

        }

      }



      $query_ajax_page_state = [];

      if ($request->query->has('ajax_page_state')) {

        $query_ajax_page_state = $this->parseAjaxPageState($request->query->all('ajax_page_state'));

        if (!empty($query_ajax_page_state)) {

          $request->query->set('ajax_page_state', $query_ajax_page_state);

        }

        else {

          $request->query->remove('ajax_page_state');

        }

      }



      // If libraries are present in both the request and the query, ensure they

      // match by merging them together.
@@ -75,7 +85,18 @@ public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = TR 
   *   changed to be uncompressed.

   */

  private function parseAjaxPageState(array $ajax_page_state): array {

    $ajax_page_state['libraries'] = UrlHelper::uncompressQueryParameter($ajax_page_state['libraries']);

    if (isset($ajax_page_state['libraries'])) {

      $libraries = explode(',', UrlHelper::uncompressQueryParameter($ajax_page_state['libraries']));

      // A library name always consists of an extension and a library name,

      // separated by a slash.

      $libraries = array_filter($libraries, static fn (string $library): bool => str_contains($library, '/'));

      if ($libraries) {

        $ajax_page_state['libraries'] = implode(',', $libraries);

      }

      else {

        unset($ajax_page_state['libraries']);

      }

    }

    return $ajax_page_state;

  }

core/modules/system/tests/src/Functional/Render/AjaxPageStateTest.php

+34 −0
Original line number Diff line number Diff line
@@ -114,4 +114,38 @@ public function testMultipleLibrariesAreNotLoaded(): void { 
    $this->assertSession()->responseContains('/core/misc/drupalSettingsLoader.js');

  }



  /**

   * Tests that an invalid library name does not inject HTML.

   */

  public function testInvalidLibraryNameDoesNotInjectHtml(): void {

    // Display all errors and warnings.

    $this->config('system.logging')->set('error_level', ERROR_REPORTING_DISPLAY_ALL)->save();



    // A value without an extension/name separator is discarded before it can

    // reach the asset system, where the raw value would be embedded in an

    // error message.

    $this->drupalGet('node', [

      'query' => [

        'ajax_page_state' => [

          'libraries' => UrlHelper::compressQueryParameter('<b>foo'),

        ],

      ],

    ]);

    $this->assertSession()->statusCodeEquals(200);

    $this->assertSession()->responseNotContains('<b>foo');



    // A value with a separator but an unknown library name is resolved as

    // extension "core" and library "<b>foo", which does not exist and is

    // silently ignored, so it cannot generate error output either.

    $this->drupalGet('node', [

      'query' => [

        'ajax_page_state' => [

          'libraries' => UrlHelper::compressQueryParameter('core/<b>foo'),

        ],

      ],

    ]);

    $this->assertSession()->statusCodeEquals(200);

    $this->assertSession()->responseNotContains('<b>foo');

  }



}

core/tests/Drupal/Tests/Core/StackMiddleware/AjaxPageStateTest.php

+28 −8
Original line number Diff line number Diff line
@@ -61,32 +61,52 @@ public function testHandle(?string $query_libraries, ?string $request_libraries, 
   * Provides data for testHandle().

   */

  public static function providerHandle(): array {

    $foo_bar = UrlHelper::compressQueryParameter('foo,bar');

    $foo_baz = UrlHelper::compressQueryParameter('foo,baz');

    $foo_bar = UrlHelper::compressQueryParameter('core/foo,core/bar');

    $foo_baz = UrlHelper::compressQueryParameter('core/foo,core/baz');

    $data = [];

    $data['only query'] = [

      $foo_bar,

      NULL,

      'foo,bar',

      'core/foo,core/bar',

      NULL,

    ];

    $data['only request'] = [

      NULL,

      $foo_bar,

      NULL,

      'foo,bar',

      'core/foo,core/bar',

    ];

    $data['matching'] = [

      $foo_bar,

      $foo_bar,

      'foo,bar',

      'foo,bar',

      'core/foo,core/bar',

      'core/foo,core/bar',

    ];

    $data['different'] = [

      $foo_baz,

      $foo_bar,

      'foo,bar,baz',

      'foo,bar,baz',

      'core/foo,core/bar,core/baz',

      'core/foo,core/bar,core/baz',

    ];

    $data['invalid libraries discarded'] = [

      UrlHelper::compressQueryParameter('<b>foo'),

      NULL,

      NULL,

      NULL,

    ];

    $data['mixed valid and invalid'] = [

      UrlHelper::compressQueryParameter('core/drupal,<b>foo'),

      NULL,

      'core/drupal',

      NULL,

    ];

    // A name containing a slash is structurally valid; the middleware does not

    // need to filter it because the asset system ignores unknown libraries.

    $data['slash-containing name preserved'] = [

      UrlHelper::compressQueryParameter('core/<b>foo'),

      NULL,

      'core/<b>foo',

      NULL,

    ];

    return $data;

  }