fix: protect internal session variables against user overwrite#10294
fix: protect internal session variables against user overwrite#10294gr8man wants to merge 6 commits into
Conversation
michalsn
left a comment
There was a problem hiding this comment.
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.
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
Description
Currently, the
Sessionclass 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_varsor__ci_last_regenerateparameters.Depending on the injected payload, this can lead to:
__ci_varsis set to a string, the next session initialization will throw aTypeErrorwheninitVars()attempts to iterate over it withforeach. This effectively breaks the session and locks the user out until the session expires or cookies are cleared.__ci_varsis crafted as an array, it can trick the garbage collection ininitVars()intounset()-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: