Skip to content

fix: protect internal session variables against user overwrite#10294

Open
gr8man wants to merge 6 commits into
codeigniter4:developfrom
gr8man:fix-session-ci-vars-protection
Open

fix: protect internal session variables against user overwrite#10294
gr8man wants to merge 6 commits into
codeigniter4:developfrom
gr8man:fix-session-ci-vars-protection

Conversation

@gr8man

@gr8man gr8man commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Description
Currently, the Session class lacks protection against overwriting internal framework session keys. If a developer sets session values directly from user input (e.g., using $session->set($this->request->getPost())), a malicious user can inject the __ci_vars or __ci_last_regenerate parameters.

Depending on the injected payload, this can lead to:

  • Denial of Service (DoS): If __ci_vars is set to a string, the next session initialization will throw a TypeError when initVars() attempts to iterate over it with foreach. This effectively breaks the session and locks the user out until the session expires or cookies are cleared.
  • Session Tampering: If __ci_vars is crafted as an array, it can trick the garbage collection in initVars() into unset()-ting arbitrary session keys within the user's session.

This PR adds a straightforward validation rule inside the set() and __set() methods to silently ignore setting any keys that start with the framework's internal prefix __ci_.

Note: The built-in methods for handling flashdata and tempdata (like markAsFlashdata) modify $_SESSION['__ci_vars'] directly as superglobals and are completely unaffected by this change. It ensures full backwards compatibility.

Checklist:

  • Secure coding practices
  • Tests have been added to cover this change
  • All tests pass

@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.

In this scenario, which should be very unlikely, overwriting __ci_* values would probably be the smallest problem. At that point, the application would already have allowed users to control session keys. Depending on how the application uses session data, this could also include application-level keys such as user_id, role, is_admin, etc.

Not sure what others will say, but if we want to restrict these, then I would target the 4.8 branch. Besides skipping, we should also log a warning so the skipped values are not silently ignored.

This would still be backward-compatible in intent, since applications should not use framework-reserved __ci_* keys, but it is not strictly behavior-compatible because those keys are currently accepted by the public session API.

Comment thread system/Session/Session.php
gr8man and others added 2 commits June 9, 2026 23:52
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants