fix: #3524971 Defend against \GuzzleHttp\Cookie\FileCookieJar gadget chain (b05412fc) · Commits · project / drupal

composer.lock

+3 −2

Original line number	Diff line number	Diff line
		@@ -368,7 +368,7 @@
		"dist": {
		"type": "path",
		"url": "core",
		"reference": "5903f5e2aa76522e01d0fa539fdec0d7217585ad"
		"reference": "cd0e310c160ead876a00877771d62624d704a312"
		},
		"require": {
		"asm89/stack-cors": "^2.3",
		@@ -530,7 +530,8 @@
		"lib/Drupal/Component/Datetime/Time.php"
		],
		"files": [
		"includes/bootstrap.inc"
		"includes/bootstrap.inc",
		"includes/guzzle_file_cookie_jar_shim.php"
		]
		},
		"scripts": {

core/composer.json

+2 −1

Original line number	Diff line number	Diff line
		@@ -126,7 +126,8 @@
		"lib/Drupal/Component/Datetime/Time.php"
		],
		"files": [
		"includes/bootstrap.inc"
		"includes/bootstrap.inc",
		"includes/guzzle_file_cookie_jar_shim.php"
		]
		},
		"bin": [

core/includes/guzzle_file_cookie_jar_shim.php

0 → 100644

+147 −0

Original line number	Diff line number	Diff line
		<?php

		/**
		* Provides a hardened replacement for \GuzzleHttp\Cookie\FileCookieJar.
		*
		* Upstream hardening landed for Guzzle 8.0 but is not included in Guzzle 7.
		* The body of the class below is a verbatim backport of the hardened Guzzle
		* 8.0 implementation.
		*
		* @see https://github.com/guzzle/guzzle/pull/3334
		*
		* @todo Remove when Drupal updates to Guzzle 8.
		*/

		namespace GuzzleHttp\Cookie;

		use GuzzleHttp\Utils;

		// phpcs:ignoreFile

		// Only define the class if the real one has not already been loaded.
		if (!class_exists(FileCookieJar::class, false)) {

		/**
		* Persists non-session cookies using a JSON formatted file
		*/
		class FileCookieJar extends CookieJar
		{
		/**
		* @var string filename
		*/
		private string $filename;

		/**
		* @var bool Control whether to persist session cookies or not.
		*/
		private bool $storeSessionCookies;

		/**
		* @var bool Whether to save the cookie jar on destruction.
		*
		* Disabled by __wakeup() to prevent FileCookieJar from being used as a
		* PHP object injection file-write gadget when an application unserializes
		* attacker-controlled data.
		*/
		private bool $autoSave = true;

		/**
		* Create a new FileCookieJar object
		*
		* @param string $cookieFile File to store the cookie data
		* @param bool $storeSessionCookies Set to true to store session cookies
		* in the cookie jar.
		*
		* @throws \RuntimeException if the file cannot be found or created
		*/
		public function __construct(string $cookieFile, bool $storeSessionCookies = false)
		{
		parent::__construct();
		$this->filename = $cookieFile;
		$this->storeSessionCookies = $storeSessionCookies;

		if (\file_exists($cookieFile)) {
		$this->load($cookieFile);
		}
		}

		/**
		* Saves the file when shutting down
		*/
		public function __destruct()
		{
		if ($this->autoSave) {
		$this->save($this->filename);
		}
		}

		/**
		* Disable automatic persistence after unserialization.
		*/
		public function __wakeup(): void
		{
		$this->autoSave = false;
		}

		/**
		* Saves the cookies to a file.
		*
		* @param string $filename File to save
		*
		* @throws \RuntimeException if the file cannot be found or created
		*/
		public function save(string $filename): void
		{
		$json = [];
		/** @var SetCookie $cookie */
		foreach ($this as $cookie) {
		if (CookieJar::shouldPersist($cookie, $this->storeSessionCookies)) {
		$json[] = $cookie->toArray();
		}
		}

		$jsonStr = Utils::jsonEncode($json, \JSON_HEX_TAG);
		if (false === \file_put_contents($filename, $jsonStr, \LOCK_EX)) {
		throw new \RuntimeException("Unable to save file {$filename}");
		}
		}

		/**
		* Load cookies from a JSON formatted file.
		*
		* Old cookies are kept unless overwritten by newly loaded ones.
		*
		* @param string $filename Cookie file to load.
		*
		* @throws \RuntimeException if the file cannot be loaded.
		*/
		public function load(string $filename): void
		{
		$json = \file_get_contents($filename);
		if (false === $json) {
		throw new \RuntimeException("Unable to load file {$filename}");
		}
		if ($json === '') {
		return;
		}

		$data = Utils::jsonDecode($json, true);
		if (\is_array($data)) {
		foreach ($data as $cookie) {
		if (!\is_array($cookie)) {
		throw new \RuntimeException("Invalid cookie file: {$filename}");
		}

		try {
		$this->setCookie(new SetCookie($cookie));
		} catch (\InvalidArgumentException $e) {
		throw new \RuntimeException("Invalid cookie file: {$filename}", 0, $e);
		}
		}
		} elseif (\is_scalar($data) && !empty($data)) {
		throw new \RuntimeException("Invalid cookie file: {$filename}");
		}
		}
		}

		}

core/tests/Drupal/Tests/Component/Cookie/FileCookieJarShimTest.php

0 → 100644

+152 −0

Original line number	Diff line number	Diff line
		<?php

		declare(strict_types=1);

		namespace Drupal\Tests\Component\Cookie;

		use GuzzleHttp\Cookie\FileCookieJar;
		use GuzzleHttp\Cookie\SetCookie;
		use PHPUnit\Framework\Attributes\Group;
		use PHPUnit\Framework\TestCase;

		// cspell:ignore phpggc

		/**
		* Tests the hardened backport of \GuzzleHttp\Cookie\FileCookieJar.
		*
		* The shim keeps full FileCookieJar functionality but ports the Guzzle 8.0
		* hardening (an $autoSave flag that __wakeup() disables and __destruct()
		* checks) so an instance restored from attacker-controlled serialized data
		* cannot be used as a file-write gadget.
		*
		* @see core/includes/guzzle_file_cookie_jar_shim.php
		* @see https://www.drupal.org/project/drupal/issues/3524971
		* @see https://github.com/guzzle/guzzle/pull/3334
		*/
		#[Group('Cookie')]
		class FileCookieJarShimTest extends TestCase {

		/**
		* The path used for cookie storage during a test.
		*
		* @var string
		*/
		protected string $file;

		/**
		* {@inheritdoc}
		*/
		protected function setUp(): void {
		parent::setUp();
		$this->file = tempnam(sys_get_temp_dir(), 'drupal_file_cookie_jar_');
		// Start from a non-existent file so the constructor does not load.
		unlink($this->file);
		}

		/**
		* {@inheritdoc}
		*/
		protected function tearDown(): void {
		if (file_exists($this->file)) {
		unlink($this->file);
		}
		parent::tearDown();
		}

		/**
		* Confirms the hardened replacement (with __wakeup) is the class in use.
		*/
		public function testHardenedClassIsLoaded(): void {
		$reflection = new \ReflectionClass(FileCookieJar::class);
		$this->assertTrue(
		$reflection->hasMethod('__wakeup'),
		'The hardened FileCookieJar must define __wakeup(); its presence indicates the core backport is loaded instead of the unhardened Guzzle 7 class.'
		);
		}

		/**
		* The jar must retain full functionality: auto-save on destruction.
		*/
		public function testLegitimateInstanceAutoSavesOnDestruction(): void {
		$jar = new FileCookieJar($this->file, TRUE);
		$jar->setCookie(new SetCookie([
		'Name' => 'sid',
		'Value' => 'abc',
		'Domain' => 'example.com',
		'Expires' => time() + 3600,
		]));
		// Destroying a normally-constructed instance should persist cookies.
		unset($jar);
		gc_collect_cycles();

		$this->assertFileExists($this->file, 'A normally-constructed FileCookieJar auto-saves on destruction.');

		$reloaded = new FileCookieJar($this->file, TRUE);
		$this->assertCount(1, $reloaded, 'Persisted cookies are reloaded by the constructor.');
		}

		/**
		* Saved files must JSON-escape tag characters (defense in depth).
		*/
		public function testSaveEscapesTagCharacters(): void {
		$jar = new FileCookieJar($this->file, TRUE);
		$jar->setCookie(new SetCookie([
		'Name' => 'x',
		'Value' => '<?php phpinfo(); ?>',
		'Domain' => 'example.com',
		'Expires' => time() + 3600,
		]));
		$jar->save($this->file);

		$contents = file_get_contents($this->file);
		$this->assertStringNotContainsString('<?php', $contents, 'A literal PHP open tag must never be written to the cookie file.');
		$this->assertStringContainsString('\u003C', $contents, 'Tag characters must be hex-escaped via JSON_HEX_TAG.');
		}

		/**
		* An unserialized jar must not auto-save (the gadget chain sink).
		*
		* This is the key security assertion: __wakeup() disables auto-save so that
		* destroying an unserialized FileCookieJar with an attacker-controlled
		* filename performs no file write.
		*/
		public function testUnserializedInstanceDoesNotAutoSave(): void {
		// Payload uses similar technique to phpggc's --public-properties to avoid
		// null bytes.
		$payload = sprintf(
		'O:%d:"%s":2:{s:8:"filename";s:%d:"%s";s:19:"storeSessionCookies";b:1;}',
		strlen(FileCookieJar::class),
		FileCookieJar::class,
		strlen($this->file),
		$this->file,
		);

		$object = unserialize($payload);

		$this->assertInstanceOf(FileCookieJar::class, $object);

		$reflection = new \ReflectionObject($object);

		// Confirm the payload actually populated the private $filename, i.e. the
		// gadget is armed and would write to $this->file if not for __wakeup().
		$this->assertSame(
		$this->file,
		$reflection->getProperty('filename')->getValue($object),
		'The payload set the private $filename — the gadget is armed.'
		);
		$this->assertFalse(
		$reflection->getProperty('autoSave')->getValue($object),
		'__wakeup() must disable auto-save on unserialized instances.'
		);

		// Force destruction.
		unset($object);
		gc_collect_cycles();

		$this->assertFileDoesNotExist(
		$this->file,
		'Destroying an unserialized FileCookieJar must not write to the attacker-controlled path.'
		);
		}

		}