Skip to content

fix: add int type hint to gc() in all session handlers - #10418

Open
gr8man wants to merge 1 commit into
codeigniter4:4.8from
gr8man:fix/session-gc-sqli-type-hint
Open

fix: add int type hint to gc() in all session handlers#10418
gr8man wants to merge 1 commit into
codeigniter4:4.8from
gr8man:fix/session-gc-sqli-type-hint

Conversation

@gr8man

@gr8man gr8man commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Description
Added int type hint to $max_lifetime in all 6 session handlers' gc() to match SessionHandlerInterface. This prevents SQL injection from non-integer input by rejecting it with a TypeError.

Affected handlers: ArrayHandler, DatabaseHandler, FileHandler, MemcachedHandler, PostgreHandler, RedisHandler.

Checklist:

  • Securely signed commits
  • PHPDoc blocks updated
  • User guide updated
  • Conforms to style guide

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite an unrealistic scenario, but still a valid fix.

Please add a changelog entry.

Comment thread tests/system/Session/Handlers/Database/AbstractHandlerTestCase.php Outdated
gr8man added a commit to gr8man/CodeIgniter4 that referenced this pull request Jul 20, 2026
- Add Unreleased section to CHANGELOG.md with PR codeigniter4#10418 entry
- Consolidate testGCRejectsNonInt to test 6 non-int types in one method
- Remove unrelated testGCWithZero and testGCWithNegativeValue tests
- Add v4.7.5 RST changelog entry for session gc type hint
@gr8man
gr8man force-pushed the fix/session-gc-sqli-type-hint branch from 4079bbb to fc5d53c Compare July 20, 2026 18:09

@michalsn michalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rebase into the 4.8 branch, as this will be a more appropriate release for these changes. Also, please change this method signature for all handlers.

$this->fail('TypeError expected for value: ' . gettype($malicious));
} catch (TypeError) {
$this->addToAssertionCount(1);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use DataProvider instead of a loop.


- **Content Security Policy:** Fixed a bug where empty ``Content-Security-Policy``, ``Content-Security-Policy-Report-Only``, and ``Reporting-Endpoints`` response headers were generated when no corresponding values existed.
- **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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add something like this under the Method Signature Changes section:

- **Session:** The ``$max_lifetime`` parameter of the following ``gc()`` methods
  now has the native ``int`` type, matching ``SessionHandlerInterface``.

You can also list all affected handlers - this will match other entries.

@paulbalandan paulbalandan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a refactor, and as mentioned should be targeting 4.8 branch instead.

$this->assertSame(1, $handler->gc(3600));
}

public function testGCRejectsNonInt(): void

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests a PHP feature, not the method itself. Thus, the test is unnecessary.

@michalsn michalsn added needs rework Changes requested by reviewer that are still pending wrong branch PRs sent to wrong branch labels Jul 22, 2026
Add int type hint to $max_lifetime in gc() for all 6 session handlers
to match SessionHandlerInterface, preventing SQL injection from
non-integer input.

Includes changelog entry in v4.8.0.rst under Method Signature Changes.
@gr8man
gr8man force-pushed the fix/session-gc-sqli-type-hint branch from 2124cc3 to e4bb6fd Compare July 29, 2026 20:10
@github-actions github-actions Bot added the github_actions Pull requests that update Github_actions code label Jul 29, 2026
@gr8man
gr8man changed the base branch from develop to 4.8 July 29, 2026 20:11
@gr8man gr8man changed the title fix: add int type hint to gc() in session database handlers fix: add int type hint to gc() in all session handlers Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github_actions Pull requests that update Github_actions code needs rework Changes requested by reviewer that are still pending wrong branch PRs sent to wrong branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants