Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelogs/v4.7.7.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Release Date: Unreleased
- **CORS:** Fixed a bug where `Cors::hasResponseHeaders()` matched header substrings via `str_contains()`, causing false positives when existing exposed headers shared a prefix with configured exposed headers.
- **HTTP:** Fixed a bug where `UploadedFile::move()` and `UploadedFile::store()` on Windows could store a file under a different name than `getName()` reported, or write its contents to an NTFS alternate data stream. Filenames ending in a period or space or containing a colon are now rejected, including explicitly supplied names.
- **Security:** Fixed a bug where `Security::__construct()` threw a `TypeError` when the CSRF cookie was an array, causing an HTTP 500 response for affected requests.
- **Session:** Fixed a bug where `PostgreHandler::lockSession()` and `releaseLock()` interpolated unescaped session IDs and IP addresses into SQL queries. They are now escaped with `BaseConnection::escape()` and executed with `simpleQuery()`.
- **Worker Mode:** Fixed a bug where every request that ended with an uncaught exception, including a `404 Page Not Found` without a 404 override, called `exit()` in the exception handler. This ended the worker script, so FrankenPHP booted a new one, and the worker loop skipped its cleanup after the request. The exception handler now returns to the worker loop.

See the repo's [CHANGELOG.md](https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md) for a complete list of bugs fixed.
4 changes: 3 additions & 1 deletion system/Session/Handlers/Database/PostgreHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,9 @@ public function gc($max_lifetime): false|int
*/
protected function lockSession(string $sessionID): bool
{
$arg = "hashtext('{$sessionID}')" . ($this->matchIP ? ", hashtext('{$this->ipAddress}')" : '');
$arg = 'hashtext(' . $this->db->escape($sessionID) . ')'
. ($this->matchIP ? ', hashtext(' . $this->db->escape($this->ipAddress) . ')' : '');

if ($this->db->simpleQuery("SELECT pg_advisory_lock({$arg})") !== false) {
$this->lock = $arg;

Expand Down
142 changes: 142 additions & 0 deletions tests/system/Session/Handlers/Database/PostgreHandlerMockTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
<?php

declare(strict_types=1);

/**
* This file is part of CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <admin@codeigniter.com>
*
* For the full copyright and license information, please view
* the LICENSE file that was distributed with this source code.
*/

namespace CodeIgniter\Session\Handlers\Database;

use CodeIgniter\Events\Events;
use CodeIgniter\Test\CIUnitTestCase;
use CodeIgniter\Test\Mock\MockConnection;
use CodeIgniter\Test\Mock\MockResult;
use Config\Session as SessionConfig;
use PHPUnit\Framework\Attributes\Group;

/**
* @internal
*/
#[Group('Others')]
final class PostgreHandlerMockTest extends CIUnitTestCase
{
private const IP_ADDRESS = "192.168.1.1' OR '1'='1";
private const SESSION_ID = "sess'id; DROP TABLE users; --";

private function createHandler(bool $matchIP, MockConnection $mockDb): PostgreHandler
{
$config = new SessionConfig();
$config->driver = PostgreHandler::class;
$config->savePath = 'ci_sessions';
$config->matchIP = $matchIP;

$handler = new PostgreHandler($config, self::IP_ADDRESS);
$this->setPrivateProperty($handler, 'db', $mockDb);

return $handler;
}

private function createMockDb(): PostgreHandlerMockConnection
{
return new PostgreHandlerMockConnection(['DBDriver' => 'Postgre']);
}

public function testLockAndReleaseSessionWithoutMatchIP(): void
{
$db = $this->createMockDb();
$handler = $this->createHandler(false, $db);

$queryCount = 0;
$listener = static function () use (&$queryCount): void {
$queryCount++;
};
Events::on('DBQuery', $listener);

$lockSession = self::getPrivateMethodInvoker($handler, 'lockSession');
$releaseLock = self::getPrivateMethodInvoker($handler, 'releaseLock');

$this->assertTrue($lockSession(self::SESSION_ID));
$this->assertSame(
"hashtext('sess''id; DROP TABLE users; --')",
self::getPrivateProperty($handler, 'lock'),
);

$this->assertTrue($releaseLock());
$this->assertFalse(self::getPrivateProperty($handler, 'lock'));

Events::removeListener('DBQuery', $listener);

$this->assertSame(0, $queryCount);
$this->assertSame([
"SELECT pg_advisory_lock(hashtext('sess''id; DROP TABLE users; --'))",
"SELECT pg_advisory_unlock(hashtext('sess''id; DROP TABLE users; --'))",
], $db->executed);

$this->assertStringNotContainsString(self::SESSION_ID, $db->executed[0]);
$this->assertStringNotContainsString(self::SESSION_ID, $db->executed[1]);
}

public function testLockAndReleaseSessionWithMatchIP(): void
{
$db = $this->createMockDb();
$handler = $this->createHandler(true, $db);

$queryCount = 0;
$listener = static function () use (&$queryCount): void {
$queryCount++;
};
Events::on('DBQuery', $listener);

$lockSession = self::getPrivateMethodInvoker($handler, 'lockSession');
$releaseLock = self::getPrivateMethodInvoker($handler, 'releaseLock');

$this->assertTrue($lockSession(self::SESSION_ID));
$this->assertSame(
"hashtext('sess''id; DROP TABLE users; --'), hashtext('192.168.1.1'' OR ''1''=''1')",
self::getPrivateProperty($handler, 'lock'),
);

$this->assertTrue($releaseLock());
$this->assertFalse(self::getPrivateProperty($handler, 'lock'));

Events::removeListener('DBQuery', $listener);

$this->assertSame(0, $queryCount);
$this->assertSame([
"SELECT pg_advisory_lock(hashtext('sess''id; DROP TABLE users; --'), hashtext('192.168.1.1'' OR ''1''=''1'))",
"SELECT pg_advisory_unlock(hashtext('sess''id; DROP TABLE users; --'), hashtext('192.168.1.1'' OR ''1''=''1'))",
], $db->executed);

$this->assertStringNotContainsString(self::SESSION_ID, $db->executed[0]);
$this->assertStringNotContainsString(self::IP_ADDRESS, $db->executed[0]);
}
}

/**
* Mock connection that records the raw SQL passed to simpleQuery().
*
* PostgreHandler uses simpleQuery() with escaped values, so the queries are not
* bound and must be captured as they are sent to the database.
*
* @internal
*/
final class PostgreHandlerMockConnection extends MockConnection
{
/**
* @var list<string>
*/
public array $executed = [];

public function simpleQuery(string $sql): MockResult
{
$this->executed[] = $sql;

return new MockResult($this->connID, $this->resultID);
}
}
Loading