From 0b34a378fefd1bedf03190d6bd7c281b474475ec Mon Sep 17 00:00:00 2001 From: Bogdan Date: Mon, 20 Jul 2026 00:57:44 +0200 Subject: [PATCH 1/5] fix: add int type hint to gc() to prevent SQL injection - Add int type hint to parameter in DatabaseHandler::gc() and PostgreHandler::gc() to match PHP SessionHandlerInterface - Change return type to int|false to match the interface - Add test for TypeError on non-int input (SQLi protection) - Add edge case tests for gc(0) and gc(-1) Fixes potential SQL injection via unescaped in INTERVAL clause with = false. --- .../Handlers/Database/PostgreHandler.php | 2 +- system/Session/Handlers/DatabaseHandler.php | 2 +- .../Database/AbstractHandlerTestCase.php | 20 +++++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/system/Session/Handlers/Database/PostgreHandler.php b/system/Session/Handlers/Database/PostgreHandler.php index 1471d44d1860..5daf382b6ee0 100644 --- a/system/Session/Handlers/Database/PostgreHandler.php +++ b/system/Session/Handlers/Database/PostgreHandler.php @@ -59,7 +59,7 @@ protected function prepareData(string $data): string * @param int $max_lifetime Sessions that have not updated * for the last max_lifetime seconds will be removed. */ - public function gc($max_lifetime): false|int + public function gc(int $max_lifetime): int|false { $separator = '\''; $interval = implode($separator, ['', "{$max_lifetime} second", '']); diff --git a/system/Session/Handlers/DatabaseHandler.php b/system/Session/Handlers/DatabaseHandler.php index c4ce2b8222e7..0be7af3ff318 100644 --- a/system/Session/Handlers/DatabaseHandler.php +++ b/system/Session/Handlers/DatabaseHandler.php @@ -276,7 +276,7 @@ public function destroy($id): bool * * @return false|int Returns the number of deleted sessions on success, or false on failure. */ - public function gc($max_lifetime): false|int + public function gc(int $max_lifetime): int|false { return $this->db->table($this->table)->where( 'timestamp <', diff --git a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php index 743508d7fe55..58d30421bdbc 100644 --- a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php +++ b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php @@ -122,4 +122,24 @@ public function testGC(): void $handler = $this->getInstance(); $this->assertSame(1, $handler->gc(3600)); } + + public function testGCRejectsNonInt(): void + { + $handler = $this->getInstance(); + + $this->expectException(\TypeError::class); + $handler->gc('malicious; DROP TABLE sessions; --'); + } + + public function testGCWithZero(): void + { + $handler = $this->getInstance(); + $this->assertSame(1, $handler->gc(0)); + } + + public function testGCWithNegativeValue(): void + { + $handler = $this->getInstance(); + $this->assertSame(1, $handler->gc(-1)); + } } From e9bc894ce49453fd049cc7d480cead1fec5838b5 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Mon, 20 Jul 2026 01:00:42 +0200 Subject: [PATCH 2/5] fix: normalize return type and add PHPStan suppression for intentional type violation test --- system/Session/Handlers/Database/PostgreHandler.php | 2 +- system/Session/Handlers/DatabaseHandler.php | 2 +- .../Session/Handlers/Database/AbstractHandlerTestCase.php | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/system/Session/Handlers/Database/PostgreHandler.php b/system/Session/Handlers/Database/PostgreHandler.php index 5daf382b6ee0..37ab7e6dfdc1 100644 --- a/system/Session/Handlers/Database/PostgreHandler.php +++ b/system/Session/Handlers/Database/PostgreHandler.php @@ -59,7 +59,7 @@ protected function prepareData(string $data): string * @param int $max_lifetime Sessions that have not updated * for the last max_lifetime seconds will be removed. */ - public function gc(int $max_lifetime): int|false + public function gc(int $max_lifetime): false|int { $separator = '\''; $interval = implode($separator, ['', "{$max_lifetime} second", '']); diff --git a/system/Session/Handlers/DatabaseHandler.php b/system/Session/Handlers/DatabaseHandler.php index 0be7af3ff318..6ddd8a3c0d9b 100644 --- a/system/Session/Handlers/DatabaseHandler.php +++ b/system/Session/Handlers/DatabaseHandler.php @@ -276,7 +276,7 @@ public function destroy($id): bool * * @return false|int Returns the number of deleted sessions on success, or false on failure. */ - public function gc(int $max_lifetime): int|false + public function gc(int $max_lifetime): false|int { return $this->db->table($this->table)->where( 'timestamp <', diff --git a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php index 58d30421bdbc..e997836862f7 100644 --- a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php +++ b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php @@ -19,6 +19,7 @@ use CodeIgniter\Test\ReflectionHelper; use Config\Database as DatabaseConfig; use Tests\Support\Database\Seeds\CITestSeeder; +use TypeError; /** * @internal @@ -127,8 +128,8 @@ public function testGCRejectsNonInt(): void { $handler = $this->getInstance(); - $this->expectException(\TypeError::class); - $handler->gc('malicious; DROP TABLE sessions; --'); + $this->expectException(TypeError::class); + $handler->gc('malicious; DROP TABLE sessions; --'); // @phpstan-ignore argument.type } public function testGCWithZero(): void From 5a53402a435ba9b06cd9acaa5a9d0a6969ba7018 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Mon, 20 Jul 2026 01:18:44 +0200 Subject: [PATCH 3/5] test: add SQLi rejection and edge-case tests for gc() type safety --- .../Session/Handlers/Database/AbstractHandlerTestCase.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php index e997836862f7..1d3ac3e72012 100644 --- a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php +++ b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php @@ -126,10 +126,11 @@ public function testGC(): void public function testGCRejectsNonInt(): void { - $handler = $this->getInstance(); + $handler = $this->getInstance(); + $malicious = 'malicious; DROP TABLE sessions; --'; $this->expectException(TypeError::class); - $handler->gc('malicious; DROP TABLE sessions; --'); // @phpstan-ignore argument.type + $handler->gc($malicious); // @phpstan-ignore argument.type } public function testGCWithZero(): void From fc5d53c88d0ee31140307ebdd4b90488995eb10a Mon Sep 17 00:00:00 2001 From: Bogdan Date: Mon, 20 Jul 2026 20:06:40 +0200 Subject: [PATCH 4/5] docs: add changelog entry and improve SQLi rejection tests - Add v4.7.5 RST changelog entry for session gc type hint - Consolidate testGCRejectsNonInt to test 6 non-int types in one method - Remove unrelated testGCWithZero and testGCWithNegativeValue tests --- .../Database/AbstractHandlerTestCase.php | 24 +++++++------------ user_guide_src/source/changelogs/v4.7.5.rst | 1 + 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php index 1d3ac3e72012..22c1c8c5afdc 100644 --- a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php +++ b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php @@ -18,6 +18,7 @@ use CodeIgniter\Test\DatabaseTestTrait; use CodeIgniter\Test\ReflectionHelper; use Config\Database as DatabaseConfig; +use stdClass; use Tests\Support\Database\Seeds\CITestSeeder; use TypeError; @@ -125,23 +126,16 @@ public function testGC(): void } public function testGCRejectsNonInt(): void - { - $handler = $this->getInstance(); - $malicious = 'malicious; DROP TABLE sessions; --'; - - $this->expectException(TypeError::class); - $handler->gc($malicious); // @phpstan-ignore argument.type - } - - public function testGCWithZero(): void { $handler = $this->getInstance(); - $this->assertSame(1, $handler->gc(0)); - } - public function testGCWithNegativeValue(): void - { - $handler = $this->getInstance(); - $this->assertSame(1, $handler->gc(-1)); + foreach (['malicious; DROP TABLE sessions; --', null, 1.5, [], new stdClass(), true] as $malicious) { + try { + $handler->gc($malicious); // @phpstan-ignore argument.type + $this->fail('TypeError expected for value: ' . gettype($malicious)); + } catch (TypeError) { + // Expected — type hint blocks SQL injection + } + } } } diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index c4bb7900e07a..a7b672901ab6 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -31,6 +31,7 @@ Bugs Fixed ********** - **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors. +- **Session:** Added ``int`` type hint to ``DatabaseHandler::gc()`` and ``PostgreHandler::gc()`` to prevent SQL injection from non-integer ``$max_lifetime`` values. See the repo's `CHANGELOG.md `_ From 2124cc31ab72e2c5f5c228979e22c1cfb651dcd7 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Mon, 20 Jul 2026 20:22:21 +0200 Subject: [PATCH 5/5] test: fix risky test - add assertion count to testGCRejectsNonInt --- .../Session/Handlers/Database/AbstractHandlerTestCase.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php index 22c1c8c5afdc..05612dd39bcb 100644 --- a/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php +++ b/tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php @@ -134,7 +134,7 @@ public function testGCRejectsNonInt(): void $handler->gc($malicious); // @phpstan-ignore argument.type $this->fail('TypeError expected for value: ' . gettype($malicious)); } catch (TypeError) { - // Expected — type hint blocks SQL injection + $this->addToAssertionCount(1); } } }