security: restrict AJAX during upgrade and escape installer config writes#706
Open
anonymoususer72041 wants to merge 2 commits intoopencats:masterfrom
Open
Conversation
999e9cb to
1f2c1c1
Compare
Member
|
Thanks — definitely in the right direction, but checks show installer security is not fully hardened after this. The remaining gap is that Could this PR also harden the config write paths by safely escaping the PHP string values before writing them, it could be as easy as using
That'd be ideal! |
1f2c1c1 to
ec805b8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When upgrade mode is active and
INSTALL_BLOCKis missing,ajax.phpnow only allows installer AJAX actions (install:*). All other AJAX requests are rejected early with the standard XML error response.This PR also hardens installer config writes in
modules/install/ajax/ui.phpby escaping string values before passing them toCATSUtility::changeConfigSetting(). The affected installer settings now use safe PHP string literals viavar_export($value, true)before being written toconfig.php.Motivation
During upgrades,
index.phpis already blocked whenINSTALL_BLOCKis missing, butajax.phpremained accessible. That could allow existing sessions or stale UI tabs to continue sending AJAX requests while schema migrations are running, increasing the risk of unintended changes or inconsistent state.Review also identified that several installer config values were still passed unescaped into
CATSUtility::changeConfigSetting(), which then writes them directly into executable PHP inconfig.php. Escaping those values closes that remaining installer hardening gap without changing the intended installer flow.