diff --git a/docs/README.txt b/docs/README.txt index f9bf19904..a73f0dc1e 100644 --- a/docs/README.txt +++ b/docs/README.txt @@ -1,15 +1,32 @@ -XOOPS 2.7.2 FINAL RELEASE +XOOPS 2.7.3 FINAL RELEASE -The XOOPS Development Team is pleased to announce the release of XOOPS 2.7.2. -This patch release repairs the installer so fresh installations and -re-installations complete reliably across the supported PHP 8.2 through PHP 8.5 -range. +The XOOPS Development Team is pleased to announce the release of XOOPS 2.7.3. +This release hardens security across the core, prepares XOOPS for PHP 8.6 while +remaining fully supported on PHP 8.2 through 8.5, adds SCEditor as an optional +BBCode editor, and introduces file-based debug configuration with a rotating, +redacting file logger. It also carries a series of reliability fixes proven in +production on xoops.org — search, comments, caching, and database error +handling among them. -Download XOOPS 2.7.2 from GitHub: https://github.com/XOOPS/XoopsCore27/releases +Security work in this cycle includes escaping in all five form renderers, a +shared PathGuard path-containment class behind the template-set browser and +editor, a completed session save-handler contract, a CSRF-protected logout, +filtered redirect query strings, and the removal of image.php's never-functional +remote-image branch. + +Download XOOPS 2.7.3 from GitHub: https://github.com/XOOPS/XoopsCore27/releases For full documentation on installing or upgrading XOOPS please see: https://xoops.github.io/xoops-docs/ +Upgrading from 2.7.2 +----------------------------------- +XOOPS 2.7.3 includes schema changes (among them a comments index that took a +listing query from 541ms to 0.5ms), so after copying the new files over the web +root, run the upgrade wizard. Existing sites need no mainfile.php changes: the +new debug configuration, error screen, and developer gate all read +xoops_data/data/debug.php directly. + Debugbar module ----------------------------------- The Debugbar module is no longer included in the XOOPS Core download. The @@ -25,16 +42,16 @@ the module files and run Update; uninstalling first is not required. Languages ----------------------------------- -XOOPS 2.7.2 is available in 37 community translations, maintained at: +XOOPS 2.7.3 is available in 37 community translations, maintained at: https://github.com/XoopsLanguages See docs/TRANSLATIONS.md for the full list of languages and the current release page for each language pack. Language packs are published independently, so check each release page for its declared XOOPS compatibility. -XOOPS 2.7.2 adds no new English language constants. Translators coming directly -from XOOPS 2.7.0 still have the single 2.7.1 addition to apply — see -docs/lang_diff.txt for its exact definition. +XOOPS 2.7.3 adds new English language constants: the SCEditor editor strings +introduced in RC 1 and one logout-confirmation string added in Final. See +docs/lang_diff.txt for their exact definitions. Help wanted: please help us find and fix translation errors, and help us add and review more languages. Every correction makes XOOPS better worldwide. @@ -48,10 +65,19 @@ Support Forums: https://xoops.org/modules/newbb/ Thank you ----------------------------------- -A release this size doesn't happen without contributors. Thank you to everyone who submitted pull requests, reported issues, tested beta packages, translated strings, reviewed security findings, and kept the conversation going on the forums and on GitHub through the long beta cycle. +A release this size doesn't happen without contributors. Thank you to everyone +who submitted pull requests, reported issues, tested the beta and RC packages, +translated strings, reviewed security findings, and kept the conversation going +on the forums and on GitHub throughout the 2.7.3 cycle. + +A special thank-you to CHCCD for testing the release candidates and reporting +the search and browse bugs fixed in this release (issues #161, #162, #163): +the "Show all" search results that could never render, the search.php fatal +errors from unvalidated module ids and the module_read bypass, and the +malformed browse.php Cache-Control header. * And a standing THANK-YOU to **[JetBrains](https://www.jetbrains.com/)** for the complimentary [PhpStorm](https://www.jetbrains.com/phpstorm/) licenses that power the core team's development. XOOPS Development Team -July 2026 +August 2026 diff --git a/docs/RELEASE_POST.md b/docs/RELEASE_POST.md index 89c180aab..eb0161133 100644 --- a/docs/RELEASE_POST.md +++ b/docs/RELEASE_POST.md @@ -1,80 +1,128 @@ -# XOOPS 2.7.2 Final — installer fixes +# XOOPS 2.7.3 Final — security hardening and PHP 8.6 readiness -The XOOPS Development Team is pleased to announce **XOOPS 2.7.2 Final**. This is -a focused patch release that repairs the installer so fresh installations and -re-installations complete reliably on current PHP versions. It contains no -database, template, or API changes. +The XOOPS Development Team is pleased to announce **XOOPS 2.7.3 Final**. This +release hardens security across the core, prepares XOOPS for PHP 8.6 while +remaining fully supported on PHP 8.2 through 8.5, adds SCEditor as an optional +BBCode editor, introduces file-based debug configuration with a rotating file +logger, and folds in a series of reliability fixes proven in production on +xoops.org. -Existing XOOPS 2.7.1 sites do not need to upgrade unless they intend to run the -installer again; 2.7.2 changes only the `install/` wizard and the version -string. - -Download XOOPS 2.7.2: +Download XOOPS 2.7.3: **[https://github.com/XOOPS/XoopsCore27/releases](https://github.com/XOOPS/XoopsCore27/releases)** --- -## Why 2.7.2 - -Shortly after 2.7.1 Final, a fresh install on PHP 8.2 was reported to fail -during the "Creating tables" step with a fatal `mysqli_sql_exception` -([#126](https://github.com/XOOPS/XoopsCore27/issues/126)). Investigating it -surfaced a small cluster of related installer problems affecting fresh installs, -re-installs, and the front page of a newly installed site. 2.7.2 fixes all of -them. - ---- - -## What's fixed - -### Fresh install no longer fatals on PHP 8.2+ - -`install_isInstalled()` probed for the users table before it existed. Since PHP -8.1, mysqli defaults to exception mode — and XOOPS runs on PHP 8.2 and later — -where the `@` silence operator does **not** suppress those exceptions, so the -probe threw a fatal `mysqli_sql_exception` ("Table '…_users' doesn't exist") on -every fresh install. The probe is now guarded so that only a confirmed missing -table means "not installed", and any other database error keeps the installer -locked. - -### The wizard no longer locks itself out mid-install - -The site-configuration, theme, and module-installation pages boot the full XOOPS -environment, which swaps the installer's session for XOOPS's own session store. -That hid the in-progress installation flags and caused the "This site is already -installed" lock to fire in the middle of a legitimate install. The lock now also -recognises the authenticated in-progress administrator — already established on -those pages via a signed one-time token — so the wizard runs to completion. - -### Default theme is a shipped theme - -A new install set its default theme to `xswatch4`, which is not shipped, so the -front page failed with "Theme not found". The default is now `xbootstrap5`. - -### Re-installing over an existing site works - -- `license.php`, left read-only by a previous install, is made writable again - before it is rewritten, instead of failing with "Make … Writable". -- A stale one-time install cookie from an earlier attempt is cleared and - re-authenticated, instead of aborting the wizard with "Init Error". -- Per-attempt installer key files are cleaned up at the start of a run so they - no longer accumulate. - -### Cleaner install log - -The initial-settings page ran its "does an administrator already exist?" check -on a connection that had not selected the database, logging a spurious -"No database selected" error. It now uses the database-aware query path. +## Security hardening + +Security received sustained attention across the whole 2.7.3 cycle: + +- **Form renderer escaping.** Element values are escaped in all five form + renderers through a shared trait, and JavaScript arguments are built with + `json_encode()` — the correct encoder for a JS string literal inside an HTML + attribute. +- **Template-set browser and editor contained by PathGuard.** The tplsets + browse, edit, restore, and save endpoints were hardened end-to-end — + double-decode removed, NUL bytes rejected, symlink escapes refused, backups + written atomically — and the path-containment contract now lives in a shared + `PathGuard` class pinned by a truth-table test suite against a real fixture + tree. +- **Logout requires a session token.** A bare GET could previously end a + visitor's session from any third-party page (forced-logout CSRF). A tokenless + request now renders a POST confirmation instead of acting immediately, so + every existing logout link keeps working at the cost of one extra click. +- **Redirect query strings are rebuilt, not reflected.** Eight redirect sites + appended the raw `QUERY_STRING` verbatim to their `Location` headers. The + string is now parsed and re-emitted via a shared, unit-tested helper + (`xoops_rebuildQueryString()`), so every reflected byte is RFC 3986-safe by + construction. +- **image.php's remote-image branch is closed.** With `ONLY_LOCAL_IMAGES` + disabled, the raw request URL previously reached `getimagesize()` and + `file_get_contents()` — an SSRF and phar-deserialization surface. The branch + had never worked (it truncated every rooted URL to a single character), so + nothing real is lost: it now fails closed. +- **Module manifests and the image manager.** Manifest values are escaped at + the point of output on the module admin log pages, the image category + handlers now enforce authorization and not just CSRF, and three stale + renderer copies under TinyMCE plugins were replaced with the core class. + +## Ready for PHP 8.6 + +XOOPS 2.7.3 runs on PHP 8.2 through 8.5 and is prepared for PHP 8.6: + +- The session save-handler contract is complete — `create_sid()` is + implemented ahead of its PHP 9.0 requirement, brand-new sessions survive + 8.6's `updateTimestamp()` routing, and `session.use_strict_mode` is pinned + to 1 (the 8.6 default) on today's PHP versions as well. +- Deprecations are cleared ahead of time: constructor value-returns (compile-time + deprecated in 8.6) are gone and guarded by a repository-wide test, + `is_long()` is replaced, and the deprecated `curl_close()` and + `imagedestroy()` calls are dropped. + +## Editors + +- **SCEditor 3.2.1** ships bundled as an optional BBCode editor, selectable + from the editor preference dropdown. It is deliberately locked to source + mode so existing content never passes through a WYSIWYG round-trip that + could rewrite XOOPS-specific BBCode. +- **One shared dhtml toolbar.** The same editor used to show a different + toolbar in the control panel than on the front end; all five renderers now + delegate to a single toolbar implementation with framework-neutral classes. + +## Debugging and logging + +- **File-based debug configuration.** Error display, error_reporting, and + query logging are set in one place — `xoops_data/data/debug.php` — instead + of being edited into mainfile.php. Nothing changes until an administrator + creates the file. +- **A rotating, redacting file logger** records notices, warnings, errors and + SQL with backtraces, so a blank page or an error on a redirect can still be + read afterwards. Server paths, session ids, and session rows are redacted; + control characters are stripped so log lines cannot be forged. +- **The error screen has one declared owner**, resolved deterministically, so + error-screen providers such as Whoops or Tracy no longer compete for the + seat, and a contested registration is detected and reported. + +## Reliability fixes from xoops.org production + +- One module with PHP4-style constructors no longer takes global search down + for every visitor; failing modules are contained and logged. +- The search "Show all" / "Show all by user" pages render results (and the + "no match" message) again, `search.php` no longer fatals on unvalidated + module ids, `showall` respects `module_read` permissions, and `browse.php` + sends a well-formed `Cache-Control` header — all reported by + [CHCCD](https://github.com/CHCCD) in + [#161](https://github.com/XOOPS/XoopsCore27/issues/161), + [#162](https://github.com/XOOPS/XoopsCore27/issues/162), and + [#163](https://github.com/XOOPS/XoopsCore27/issues/163). +- `xoops_getrank()` no longer fatals when no rank row matches. +- A failed query handed to a result-set method returns the documented failure + value instead of blanking the page with a TypeError. +- `Criteria` renders an empty `IN ()` list as a constant predicate instead of + invalid SQL, and preserves the caller's PHP type in IN lists. +- The unfiltered group list is memoised per request, removing roughly 48 + identical queries per page; comment listings gained an index that took one + query from 541ms to 0.5ms. +- The login redirect no longer accumulates escaped ampersands hop by hop. + +## Deprecations + +The `XOBJ_DTYPE_UNICODE_*` object datatypes are deprecated in 2.7.3 — they +url-encode on write and url-decode on read, a pre-UTF-8 workaround that bloats +storage and breaks `LIKE`/`FULLTEXT` search on modern utf8mb4 installs. This +release only reports their use; behavior is unchanged. Data migration is +planned for 2.8 and constant removal for 4.0. --- ## Upgrading -### From XOOPS 2.7.1 +### From XOOPS 2.7.2 -2.7.2 has no database or template changes and does not require the upgrade -wizard. To move an existing 2.7.1 site to 2.7.2, copy the new `htdocs/` files -over the web root. There is nothing else to do. +XOOPS 2.7.3 includes schema changes, so after copying the new `htdocs/` files +over the web root, **run the upgrade wizard**. No mainfile.php changes are +needed: the debug configuration, error screen, and developer gate all read +`xoops_data/data/debug.php` directly, so creating that file is sufficient on +its own. ### Installing fresh or from older versions @@ -87,7 +135,7 @@ Follow the standard installation and upgrade guidance: | | | |---|---| -| **PHP** | >= 8.2.0; PHP 8.4 or 8.5 recommended | +| **PHP** | >= 8.2.0; PHP 8.4 or 8.5 recommended, prepared for 8.6 | | **MySQL / MariaDB** | MySQL >= 5.7.8 or MariaDB >= 10.5; a supported MySQL 8.x or MariaDB LTS release is recommended | | **Web server** | Apache 2.4+ or nginx | @@ -95,11 +143,13 @@ Follow the standard installation and upgrade guidance: ## Translations -XOOPS 2.7.2 introduces no new or changed English language constants, so no -translation updates are required. XOOPS remains maintained in **37 community -translations** under the [XoopsLanguages](https://github.com/XoopsLanguages) -organization; see [`docs/TRANSLATIONS.md`](TRANSLATIONS.md) for the current -release page of each language. +XOOPS 2.7.3 adds new English language constants: the SCEditor editor strings +introduced in RC 1 and one logout-confirmation string added in Final — see +[`docs/lang_diff.txt`](lang_diff.txt) for their exact definitions. XOOPS +remains maintained in **37 community translations** under the +[XoopsLanguages](https://github.com/XoopsLanguages) organization; see +[`docs/TRANSLATIONS.md`](TRANSLATIONS.md) for the current release page of each +language. --- @@ -113,19 +163,26 @@ release page of each language. ## Thank you -Thank you to everyone who reported the installation problems, tested the fixes, -and helped confirm the release. Bug reports like -[#126](https://github.com/XOOPS/XoopsCore27/issues/126) make XOOPS better for -everyone. +A release this size doesn't happen without contributors. Thank you to everyone +who submitted pull requests, reported issues, tested the beta and RC packages, +translated strings, reviewed security findings, and kept the conversation going +on the forums and on GitHub throughout the 2.7.3 cycle. + +A special thank-you to [CHCCD](https://github.com/CHCCD) for testing the +release candidates and reporting the search and browse bugs fixed in this +release ([#161](https://github.com/XOOPS/XoopsCore27/issues/161), +[#162](https://github.com/XOOPS/XoopsCore27/issues/162), +[#163](https://github.com/XOOPS/XoopsCore27/issues/163)). Bug reports like +these make XOOPS better for everyone. We also thank [JetBrains](https://www.jetbrains.com/) for supporting the project with [PhpStorm](https://www.jetbrains.com/phpstorm/) licenses. -**Download XOOPS 2.7.2:** +**Download XOOPS 2.7.3:** [https://github.com/XOOPS/XoopsCore27/releases](https://github.com/XOOPS/XoopsCore27/releases) --- **The XOOPS Development Team** -July 2026 +August 2026 diff --git a/docs/RELEASE_POST_BBCODE.txt b/docs/RELEASE_POST_BBCODE.txt new file mode 100644 index 000000000..524f0b98e --- /dev/null +++ b/docs/RELEASE_POST_BBCODE.txt @@ -0,0 +1,58 @@ +[size=x-large][b]XOOPS 2.7.3 Final Released — security hardening and PHP 8.6 readiness[/b][/size] + +The XOOPS Development Team is pleased to announce [b]XOOPS 2.7.3 Final[/b]. This release hardens security across the core, prepares XOOPS for PHP 8.6 while remaining fully supported on PHP 8.2 through 8.5, adds SCEditor as an optional BBCode editor, introduces file-based debug configuration with a rotating file logger, and folds in a series of reliability fixes proven in production on xoops.org. + +[b]DOWNLOAD[/b]: You can download the release from here: https://github.com/XOOPS/XoopsCore27/releases + +[size=large][b]Security hardening[/b][/size] +* Element values escaped in all five form renderers, with JavaScript arguments built via json_encode() +* Template-set browser and editor contained by the new shared PathGuard class — double-decode removed, NUL bytes rejected, symlink escapes refused, atomic backups — pinned by a truth-table test suite +* Logout now requires a session token: a bare GET renders a POST confirmation instead of ending the session (forced-logout CSRF closed); every existing logout link keeps working +* Redirect query strings are rebuilt through a shared, unit-tested helper instead of being reflected verbatim into Location headers (eight sites) +* image.php's never-functional remote-image branch (an SSRF and phar-deserialization surface) is closed and fails closed +* Module-manifest values escaped on the module admin pages; image category handlers now enforce authorization, not just CSRF + +[size=large][b]Ready for PHP 8.6[/b][/size] +* Complete session save-handler contract: create_sid() ahead of its PHP 9.0 requirement, new sessions survive 8.6's updateTimestamp() routing, session.use_strict_mode pinned to the 8.6 default today +* Deprecations cleared ahead of time: constructor value-returns (guarded by a repository-wide test), is_long(), curl_close(), imagedestroy() + +[size=large][b]Editors[/b][/size] +* SCEditor 3.2.1 ships bundled as an optional BBCode editor, deliberately locked to source mode so existing content never passes through a WYSIWYG round-trip +* One shared dhtml toolbar for all five renderers — no more different toolbars in the control panel and the front end + +[size=large][b]Debugging and logging[/b][/size] +* File-based debug configuration in xoops_data/data/debug.php — error display, error_reporting, and query logging in one place, no mainfile.php edits; nothing changes until an administrator creates the file +* A rotating, redacting file logger records notices, warnings, errors and SQL with backtraces — server paths, session ids and session rows are redacted +* The error screen has one declared owner, so providers such as Whoops or Tracy no longer compete for the seat + +[size=large][b]Reliability fixes from production[/b][/size] +* A failing module no longer takes global search down for every visitor; the "Show all" search pages render results again; search.php validates the request and respects module_read; browse.php sends a well-formed Cache-Control header +* xoops_getrank() no longer fatals when no rank row matches; a failed query returns the documented failure value instead of blanking the page +* Criteria renders an empty IN () list as a constant predicate instead of invalid SQL +* The group list is memoised per request (~48 identical queries removed per page) and a new comments index took a listing query from 541ms to 0.5ms + +[size=large][b]Deprecations[/b][/size] +The XOBJ_DTYPE_UNICODE_* object datatypes are deprecated (notice only, behavior unchanged): they url-encode on write and url-decode on read, which bloats storage and breaks LIKE/FULLTEXT search on utf8mb4. Data migration is planned for 2.8, constant removal for 4.0. + +[size=large][b]Upgrading from 2.7.2[/b][/size] +XOOPS 2.7.3 includes schema changes, so after copying the new htdocs/ files over the web root, [b]run the upgrade wizard[/b]. No mainfile.php changes are needed. Full documentation: https://xoops.github.io/xoops-docs/ + +[size=large][b]System requirements[/b][/size] +* PHP >= 8.2.0 (PHP 8.4 or 8.5 recommended, prepared for 8.6) +* MySQL >= 5.7.8 or MariaDB >= 10.5 (a supported MySQL 8.x or MariaDB LTS recommended) +* Apache 2.4+ or nginx + +[size=large][b]Translations[/b][/size] +XOOPS 2.7.3 adds new English language constants: the SCEditor editor strings introduced in RC 1 and one logout-confirmation string added in Final — see docs/lang_diff.txt. XOOPS remains maintained in 37 community translations: https://github.com/XoopsLanguages + +[size=large][b]Thank you[/b][/size] +Thank you to everyone who submitted pull requests, reported issues, tested the beta and RC packages, translated strings, and reviewed security findings throughout the 2.7.3 cycle. + +A special thank-you to CHCCD for testing the release candidates and reporting the search and browse bugs fixed in this release (issues #161, #162, #163). Bug reports like these make XOOPS better for everyone. + +We also thank JetBrains ( https://www.jetbrains.com/ ) for supporting the project with PhpStorm licenses. + +[b]Bug reports[/b]: https://github.com/XOOPS/XoopsCore27/issues +[b]Support forums[/b]: https://xoops.org/modules/newbb/ + +The XOOPS Development Team diff --git a/docs/changelog.270.txt b/docs/changelog.270.txt index ec31e9c56..d88dcc273 100644 --- a/docs/changelog.270.txt +++ b/docs/changelog.270.txt @@ -1,5 +1,37 @@ XOOPS 2.7.x Changelog (Language changes: see: /docs/lang_diff.txt) +=================================== +2.7.3 Final 2026-08 +=================================== +Compatibility: +- replace the deprecated is_long() alias with is_int() in FunctionsUserTest, the lone repository-wide occurrence; PHP 8.6 deprecates the is_long/is_integer/is_double/doubleval type-alias functions and first-party htdocs/ code has none (mamba) +- use bare returns in the four early-exit constructors (class/file/file.php, class/uploader.php, class/xoopsform/formdhtmltextarea.php, class/xoopsform/formselectuser.php): PHP 8.6 deprecates returning a value from __construct() at compile time; new never delivered these values, so the change is behavior-preserving. Verified by token scan - 7 constructor value-returns before, 0 after - and pinned by a direct-invocation test that observes the yielded value (mamba) +- complete the session save-handler contract ahead of PHP 8.6: add create_sid() (with SessionIdInterface) so the 8.6 deprecation for object handlers lacking it never fires - PHP 9.0 makes the method mandatory; teach updateTimestamp() to insert the row a brand-new empty session is missing, because 8.6 routes such a session there instead of write(), and a bare UPDATE would never create it, leaving strict mode to reject the ID and regenerate it on every request - gated on read() having found no row for the ID, so a session destroyed by a parallel logout keeps its timestamp-only UPDATE and cannot be resurrected with stale data; pin session.use_strict_mode to 1, the 8.6 default, on 8.2-8.5 as well, reporting via E_USER_WARNING when the pin is impossible because output preceded session bootstrap (mamba) in #169 +- drop the imagedestroy() calls, deprecated in PHP 8.5 and no-ops since 8.0 (a GdImage frees itself when its last reference goes away): image.php, the image CAPTCHA renderer (whose required-GD-function check no longer lists it, and whose branch-on-destroy-failure unwinds to a plain unset), and Protector's module_icon.php; the remaining occurrences are inside the vendored TCPDF, which is upstream's to change (mamba) in #179 + +Kernel: +- deprecate the XOBJ_DTYPE_UNICODE_* object datatypes (16-21), which url-encode on write and url-decode on read — a pre-UTF-8 workaround that bloats storage and breaks LIKE/FULLTEXT search on modern utf8mb4 installs. Notice only, behavior unchanged: initVar() reports a deprecation when one is used (guarded so it is safe during early bootstrap, before the logger exists) and the constant block carries an @deprecated doc naming the non-unicode successors; data migration is planned for 2.8 and constant removal for 4.0 (mamba) + +Security: +- harden the tplsets template browser, editor, restore, and save endpoints: drop the second urldecode() (PHP already decoded the request once; re-decoding re-materialized %00 and %2f as live bytes) and reject interior NUL bytes explicitly at ingestion at all four sites - getString()'s trim() silently drops edge NULs (so such a value arrives as a valid NUL-free path and faces the same containment checks as any other input) while an interior one passes intact, verified by execution; the accessors themselves stay as they were, because Xmf's PATH filter truncates at the first byte outside its ASCII set, which would break accented or CJK theme paths and turn a Windows restore path into a bare drive letter - with realpath() inside try and ValueError caught as a backstop at all four sites (browser, editor, restore, and the save in main.php), false realpath() results refused, root-plus-separator containment so a sibling like themes2 cannot satisfy a bare prefix check, is_file() required in the editor, save, and restore paths so a directory (even the themes root via path_file=/) never reaches the file handlers, the folder listing switched to the canonical path it just validated, reads pinned to POST to match the JS callers, the template-extension allowlist enforced at the editor as well as the save, the legacy escape() dropped from the tree JS (jQuery already encodes once, so spaced and non-ASCII folder names finally reach the server intact - the removed second decode had been load-bearing for that double-encode), the editor/restore AJAX bodies switched to object form so paths containing & or + survive, restore switched to the validated relative path (ending the server-path leak into admin HTML and the Windows absolute-path special case), a failed backup copy() now aborts the save, the file-listing onclick arguments JS-encoded with json_encode() before HTML-escaping (htmlentities() alone left an apostrophe filename free to break the JS literal), the editor form kept to its single CSRF token with the restore validating it via check(false) so the save can still spend it, the hidden path_file field re-derived from the validated path, the dead file/ext fields dropped, the backup delegated to the shared xoops_write_file_atomically() helper, whose atomic rename() replaces a planted symlink at .back instead of following it (copy() follows an existing link and PHP's emulated fopen('x') follows a dangling one, both verified by execution; an attacker who could exploit the helper's residual reopen-by-name window already has write access inside themes/ and could replace the templates directly), a symlinked .back refused by the restore and its delete-then-rename replaced by a single atomic rename() so a failed restore can no longer leave neither file, the listing/editor output aligned on htmlspecialchars(ENT_QUOTES | ENT_SUBSTITUTE, UTF-8) with json_encode(JSON_INVALID_UTF8_SUBSTITUTE) for the JS arguments - without the SUBSTITUTE flags a non-UTF-8 filename produced a JS parse error or an empty editor textarea that a save would write over the real file, both verified by execution - and tpls_edit_file() slimmed to the two parameters it actually uses (mamba) in #176 +- extract the tplsets path-containment contract into the shared PathGuard class (resolveDir/resolveFile: NUL refusal with a ValueError backstop, false-realpath refusal, boundary-aware root containment so a sibling like themes2 never satisfies a prefix check, is_dir/is_file by mode, case-insensitive extension allowlist, canonical result), refactor all four endpoints onto the one implementation, and pin the contract with a truth-table PHPUnit suite that runs against a real fixture tree - traversal, NUL masks, a directory named like a stylesheet, symlink escape refused and an internal symlink canonicalized - and harden the four tpls_generate_surcharge write loops through one checked writer: fopen() verified (an unchecked false handle is a PHP 8 TypeError at fwrite()) and fwrite() compared against false so an empty template source still counts as written (mamba) in #178 +- close image.php's remote-image branch. With ONLY_LOCAL_IMAGES flipped to false the raw request URL reached getimagesize() and file_get_contents(), honoring http://, phar:// and data:// — an SSRF and phar-deserialization surface — and the branch had never worked for anyone anyway: it truncated every rooted URL to a single character (substr($imageUrl, 0, 1) where substr($imageUrl, 1) was meant), so no real-world behavior is lost. The constant stays and the branch now refuses the request, so a fork that flips it fails closed instead of fatal (mamba) in #179 +- require a valid session token to log out. profile's user.php tore the session down on a bare GET, so any third-party page could end a visitor's session at will (forced-logout CSRF). A tokenless request now renders a POST confirmation carrying the token instead of acting immediately, which keeps every existing user.php?op=logout link working — including the ones baked into cached theme and block templates — at the cost of one extra click (mamba) in #179 +- stop reflecting the raw QUERY_STRING into redirect Location headers at all eight sites (three pm preload events, four profile preload events, and profile's register.php activation redirect). PHP already blocks CRLF in headers, so this was never header splitting, but an attacker-shaped query string reflected verbatim invites cache-poisoning and phishing parameter injection; the string is now parsed with parse_str() — which mirrors how the redirect target itself reads the parameters — and re-emitted with http_build_query(), so every reflected byte is RFC 3986-safe or a valid escape by construction, hostile input becomes inert encoding instead of costing the visitor their query, and a long urlencoded xoops_redirect survives (the 2000-byte cap is header-size sanity only); the rebuild lives in include/file_safety.php as xoops_rebuildQueryString() — one implementation shared by all three call sites — and is pinned by a unit test (mamba) in #179 + +Search: +- render the search "Show all" / "Show all by user" results again: system_search.tpl tested a guard no request could satisfy ($nomatch is left unassigned when a module returns rows and set true when it does not, but the template tested isset() && != true), so neither the result list nor the no-match message had rendered since the guard was rewritten. The block now opens on empty($nomatch), the no-match message becomes a proper else branch, the optional values that become reachable again (showall, uname, time, and the pagination links) are guarded, and the xswatch5 override's guards are aligned the same way (mamba); reported by CHCCD in #161 +- validate the show-all search request before rendering: search.php built a theme and fired the header events before checking that the requested module exists, is active, is searchable, and is readable by the visitor, and its permission id checks now compare strictly. The row normaliser owns the row's shape — an integer uid (default 0) and string link and title — so a module search plugin returning loose types can no longer put a non-numeric uid into getUnameFromId() or the userinfo link; the show-all loop had been writing its uid cast to a stray key of the outer array while the row's own uid passed through untouched (mamba); reported by CHCCD in #162 + +Caching: +- send a well-formed Cache-Control header from browse.php: it emitted "maxage=", a directive HTTP does not define (RFC 9111), so caches ignored the field and the intended policy survived only through the Expires header alongside it — affecting every asset served through browse.php, including the bundled jQuery most themes load in . It now sends "public, max-age=" and drops "Pragma: public", which as a request-header directive never meant anything (mamba); reported by CHCCD in #163 + +Tests: +- guard the whole tree against constructor value-returns: a closure-aware token-scan convention test parses every first-party htdocs/ file (bundled vendor trees excluded) and fails on any value-carrying return inside __construct(), which PHP 8.6 deprecates and 9.0 forbids - widening the four-file pin from the compatibility fix into a repository-wide invariant (mamba) + +Build: +- add a pull request template whose checklist encodes what automated review actually caught on the PHP 8.6 train: a changelog entry in docs/changelog.270.txt, both-branch test coverage, standalone-runnable tests that restore every process-global they touch, no full server paths in diagnostics, explicit failure handling in production code, a description kept in sync with the implementation, and the conventional-commit dialect (mamba) in #175 + =================================== 2.7.3 RC 1 2026-08 =================================== @@ -13,10 +45,7 @@ Compatibility: - repair [code] and [quote] rendering on PHP 8.3+. highlight_string() changed in 8.3, returning "
" with real newlines where earlier versions returned "" with 
and  . The extension located its opening marker with strpos($buffer, '<?php ') and added a hard-coded 14; on 8.3+ that strpos() returns false, false + 14 evaluates to 14, and the first fourteen characters of real output were cut, leaving a block beginning with the literal text le="color: #000000">. The marker is now found with a pattern accepting either encoding and its length taken from the match (mamba) in #150 - keep code-block whitespace intact and scope the trailing-break trim to XOOPS' own boxes (mamba) in #150 - drop deprecated curl_close() calls for PHP 8.5 (mamba) in #153 -- replace the deprecated is_long() alias with is_int() in FunctionsUserTest, the lone repository-wide occurrence; PHP 8.6 deprecates the is_long/is_integer/is_double/doubleval type-alias functions and first-party htdocs/ code has none (mamba) -- use bare returns in the four early-exit constructors (class/file/file.php, class/uploader.php, class/xoopsform/formdhtmltextarea.php, class/xoopsform/formselectuser.php): PHP 8.6 deprecates returning a value from __construct() at compile time; new never delivered these values, so the change is behavior-preserving. Verified by token scan - 7 constructor value-returns before, 0 after - and pinned by a direct-invocation test that observes the yielded value (mamba) - drop ReflectionProperty::setAccessible() from the test bootstrap; a no-op since 8.1 and deprecated in 8.5, and emitted during bootstrap it contaminated the output of every test asserting on a clean stream (mamba) in #157 -- complete the session save-handler contract ahead of PHP 8.6: add create_sid() (with SessionIdInterface) so the 8.6 deprecation for object handlers lacking it never fires - PHP 9.0 makes the method mandatory; teach updateTimestamp() to insert the row a brand-new empty session is missing, because 8.6 routes such a session there instead of write(), and a bare UPDATE would never create it, leaving strict mode to reject the ID and regenerate it on every request - gated on read() having found no row for the ID, so a session destroyed by a parallel logout keeps its timestamp-only UPDATE and cannot be resurrected with stale data; pin session.use_strict_mode to 1, the 8.6 default, on 8.2-8.5 as well, reporting via E_USER_WARNING when the pin is impossible because output preceded session bootstrap (mamba) in #169 Debugging and logging: - offer the error screen to one declared owner instead of whichever module registered last. Ownership resolves in three steps, first answer wins: an explicit token in xoops_data/data/debug.php, else the token a provider recorded at install, else 'core'. Four constants publish the result on every request - XOOPS_ERROR_SCREEN_OWNER, _SOURCE, _STATUS and _MESSAGE - so a diagnostics page can answer "who owns the error screen here?" (mamba) in #157 @@ -35,8 +64,6 @@ Security: - enforce authorization on the image category create, update and delete handlers, which validated the CSRF token but not the caller's permission; the admin check governed only which controls were rendered (mamba) in #151 - remove three stale copies of core renderers under the TinyMCE image-manager plugins, which were installed as the global renderer so those screens never received core renderer changes; the endpoints now xoops_load() the core class (mamba) in #151 - guard getByDirname() on the module uninstall and update confirm screens (mamba) in #156 -- harden the tplsets template browser, editor, restore, and save endpoints: drop the second urldecode() (PHP already decoded the request once; re-decoding re-materialized %00 and %2f as live bytes) and reject interior NUL bytes explicitly at ingestion at all four sites - getString()'s trim() silently drops edge NULs (so such a value arrives as a valid NUL-free path and faces the same containment checks as any other input) while an interior one passes intact, verified by execution; the accessors themselves stay as they were, because Xmf's PATH filter truncates at the first byte outside its ASCII set, which would break accented or CJK theme paths and turn a Windows restore path into a bare drive letter - with realpath() inside try and ValueError caught as a backstop at all four sites (browser, editor, restore, and the save in main.php), false realpath() results refused, root-plus-separator containment so a sibling like themes2 cannot satisfy a bare prefix check, is_file() required in the editor, save, and restore paths so a directory (even the themes root via path_file=/) never reaches the file handlers, the folder listing switched to the canonical path it just validated, reads pinned to POST to match the JS callers, the template-extension allowlist enforced at the editor as well as the save, the legacy escape() dropped from the tree JS (jQuery already encodes once, so spaced and non-ASCII folder names finally reach the server intact - the removed second decode had been load-bearing for that double-encode), the editor/restore AJAX bodies switched to object form so paths containing & or + survive, restore switched to the validated relative path (ending the server-path leak into admin HTML and the Windows absolute-path special case), a failed backup copy() now aborts the save, the file-listing onclick arguments JS-encoded with json_encode() before HTML-escaping (htmlentities() alone left an apostrophe filename free to break the JS literal), the editor form kept to its single CSRF token with the restore validating it via check(false) so the save can still spend it, the hidden path_file field re-derived from the validated path, the dead file/ext fields dropped, the backup delegated to the shared xoops_write_file_atomically() helper, whose atomic rename() replaces a planted symlink at .back instead of following it (copy() follows an existing link and PHP's emulated fopen('x') follows a dangling one, both verified by execution; an attacker who could exploit the helper's residual reopen-by-name window already has write access inside themes/ and could replace the templates directly), a symlinked .back refused by the restore and its delete-then-rename replaced by a single atomic rename() so a failed restore can no longer leave neither file, the listing/editor output aligned on htmlspecialchars(ENT_QUOTES | ENT_SUBSTITUTE, UTF-8) with json_encode(JSON_INVALID_UTF8_SUBSTITUTE) for the JS arguments - without the SUBSTITUTE flags a non-UTF-8 filename produced a JS parse error or an empty editor textarea that a save would write over the real file, both verified by execution - and tpls_edit_file() slimmed to the two parameters it actually uses (mamba) in #176 -- extract the tplsets path-containment contract into the shared PathGuard class (resolveDir/resolveFile: NUL refusal with a ValueError backstop, false-realpath refusal, boundary-aware root containment so a sibling like themes2 never satisfies a prefix check, is_dir/is_file by mode, case-insensitive extension allowlist, canonical result), refactor all four endpoints onto the one implementation, and pin the contract with a truth-table PHPUnit suite that runs against a real fixture tree - traversal, NUL masks, a directory named like a stylesheet, symlink escape refused and an internal symlink canonicalized - and harden the four tpls_generate_surcharge write loops through one checked writer: fopen() verified (an unchecked false handle is a PHP 8 TypeError at fwrite()) and fwrite() compared against false so an empty template source still counts as written (mamba) in #178 Database: - render an empty IN list as a constant predicate. Criteria::render() emitted the literal IN (), which MySQL rejects as a syntax error; an empty set is now 1=0 for IN and 1=1 for NOT IN. Returning an empty fragment would be wrong, since CriteriaCompo drops empty fragments and the condition would silently match every row. This also fixes search.php, which built the fragment "mid IN ()" for any user whose module-read permission list was empty (mamba) in #154 @@ -50,10 +77,6 @@ Tests: - cover the error-screen seam in 36 cases, each in its own process, since the seam publishes its outcome as constants and constants are defined once per process. The fixture spec crosses that boundary as base64, because raw JSON through escapeshellarg() is silently mangled on Windows (mamba) in #157 - cover each term of the developer gate with a case that fails when that term alone is removed, six in all, and they are the only cases in that file which run the real xoops_isDeveloperRequest() rather than the fixture's stub. The fixture gained a mainfile_debug_constant key so a case can define XOOPS_DEBUG before debugconfig.php loads, which is the only way to reproduce an upgraded site. Written first as a pair; mutation testing showed the pair passed a rewrite that substituted the configuration read for the constant read, which would have silently dropped every development site running a hand-edited XOOPS_DEBUG (mamba) - stop two tests leaking state into the rest of the suite: MyTextSanitizerTest popped two handler frames believing displayTarea() installed them, when XoopsLogger::getInstance() installs them and only on its first call in the process; and LostPassSecurityTest never cleared its rate-limit cache, so with an idLimit of 3 the suite was reproducible exactly twice in one working copy and failed on the third run (mamba) in #157 -- guard the whole tree against constructor value-returns: a closure-aware token-scan convention test parses every first-party htdocs/ file (bundled vendor trees excluded) and fails on any value-carrying return inside __construct(), which PHP 8.6 deprecates and 9.0 forbids - widening the four-file pin from the compatibility fix into a repository-wide invariant (mamba) - -Build: -- add a pull request template whose checklist encodes what automated review actually caught on the PHP 8.6 train: a changelog entry in docs/changelog.270.txt, both-branch test coverage, standalone-runnable tests that restore every process-global they touch, no full server paths in diagnostics, explicit failure handling in production code, a description kept in sync with the implementation, and the conventional-commit dialect (mamba) in #175 =================================== 2.7.3 Beta 1 2026-07 diff --git a/docs/lang_diff.txt b/docs/lang_diff.txt index b936cc2ee..94a9cac7b 100644 --- a/docs/lang_diff.txt +++ b/docs/lang_diff.txt @@ -4,7 +4,14 @@ LANGUAGE DIFFERENCES Below are language differences from a version to next version. ================================ -2026/08: Version 2.7.3 +2026/08: Version 2.7.3-Final +================================ +/htdocs/language/english/user.php +- added define('_US_SURETOLOGOUT', 'Are you sure you want to log out?'); + (logout now asks for confirmation when the request carries no session token) + +================================ +2026/08: Version 2.7.3-RC1 ================================ /htdocs/class/xoopseditor/sceditor/language/english.php (new file — SCEditor editor) - added define('_XOOPS_EDITOR_SCEDITOR', 'SCEditor (BBCode)'); diff --git a/htdocs/class/captcha/image.php b/htdocs/class/captcha/image.php index c133e8eca..975b4b1e7 100644 --- a/htdocs/class/captcha/image.php +++ b/htdocs/class/captcha/image.php @@ -42,7 +42,6 @@ public function isActive() 'imagecolorallocate', 'imagefilledrectangle', 'imagejpeg', - 'imagedestroy', 'imageftbbox', ]; foreach ($required_functions as $func) { diff --git a/htdocs/class/captcha/image/scripts/image.php b/htdocs/class/captcha/image/scripts/image.php index aca34f458..039b8d955 100644 --- a/htdocs/class/captcha/image/scripts/image.php +++ b/htdocs/class/captcha/image/scripts/image.php @@ -191,10 +191,9 @@ public function createImageGd() return false; } - if (!imagedestroy($this->oImage)) { - // Log or handle the error as you see fit - return false; - } + // GdImage frees itself when the last reference goes away (PHP 8.0+); + // imagedestroy() is a no-op there and deprecated in PHP 8.5. + unset($this->oImage); return true; } @@ -225,7 +224,7 @@ public function setImageSize() } } } - imagedestroy($oImage); + unset($oImage); // frees the GdImage; imagedestroy() is deprecated in PHP 8.5 $this->height = $MaxCharHeight + 2; $this->spacing = (int)(($this->config['num_chars'] * $MaxCharWidth) / $this->config['num_chars']); @@ -272,7 +271,7 @@ public function createFromFile() } if (!empty($BackgroundImage)) { imagecopyresized($this->oImage, $BackgroundImage, 0, 0, 0, 0, imagesx($this->oImage), imagesy($this->oImage), imagesx($BackgroundImage), imagesy($BackgroundImage)); - imagedestroy($BackgroundImage); + unset($BackgroundImage); // frees the GdImage; imagedestroy() is deprecated in PHP 8.5 } else { $this->drawBars(); } diff --git a/htdocs/image.php b/htdocs/image.php index 708387d75..ec2816ac4 100644 --- a/htdocs/image.php +++ b/htdocs/image.php @@ -183,8 +183,9 @@ function imageCreateCorners($sourceImage, $radii) imagefilledrectangle($destinationImage, 0, 0, $imageWidth, $imageHeight, $alphaColor); imagecopyresampled($destinationImage, $workingImage, 0, 0, 0, 0, $imageWidth, $imageHeight, $workingWidth, $workingHeight); - // imagedestroy($sourceImage); - imagedestroy($workingImage); + // GdImage frees itself when the last reference goes away (PHP 8.0+); + // imagedestroy() is a no-op there and deprecated in PHP 8.5. + unset($workingImage); return $destinationImage; } @@ -333,19 +334,20 @@ function imageFilenameCheck($imageUrl) exitInvalidRequest(); } } else { - if ($imageUrl[0] === '/') { - $imageUrl = substr($imageUrl, 0, 1); - } - $imagePath = $imageUrl; + // The remote-image branch is gone (2.7.3): it handed the raw request URL + // to getimagesize()/file_get_contents(), honoring http://, phar:// and + // data:// (SSRF and phar-deserialization surface) — and it had never + // worked anyway, since it truncated any rooted URL to a single character + // (substr($imageUrl, 0, 1) where substr($imageUrl, 1) was meant). The + // constant stays so a fork that flips it fails closed instead of fatal. + exitInvalidRequest(); } // Get the size and MIME type of the requested image $imageFilename = basename($imagePath); // image filename $imagesize = getimagesize($imagePath); // Reject before decode if the source is unreadable or would decode to an - // excessive pixel count (M-14). The byte cap only applies to a local file — - // if ONLY_LOCAL_IMAGES is ever disabled, $imagePath may be a remote URL, where - // is_file()/filesize() do not apply (the pixel cap from getimagesize() still - // does). For a local file a stat failure is a rejection, not a pass. + // excessive pixel count (M-14). $imagePath is always a local file here (the + // remote branch above is closed), so a stat failure is a rejection, not a pass. $isLocalSource = is_file($imagePath); $srcBytes = $isLocalSource ? filesize($imagePath) : false; if (false === $imagesize @@ -712,9 +714,9 @@ function imageFilenameCheck($imageUrl) // Update $image_created_time $imageCreatedTime = time(); -// Clean up the memory -imagedestroy($sourceImage); -imagedestroy($destination_image); +// Clean up the memory — dropping the last reference frees the GdImage on +// PHP 8.0+; imagedestroy() is a no-op there and deprecated in PHP 8.5. +unset($sourceImage, $destination_image); /* * Write the just edited image into the Xoops cache diff --git a/htdocs/include/file_safety.php b/htdocs/include/file_safety.php index 4cc9db50c..da88ca6bc 100644 --- a/htdocs/include/file_safety.php +++ b/htdocs/include/file_safety.php @@ -10,6 +10,7 @@ * - xoops_chmod_quietly() — scoped-suppressed chmod() with single warning * - xoops_remove_file_quietly() — scoped-suppressed unlink() with single warning * - xoops_isLocalUrl() — strict same-origin check (scheme/host/port) for redirects + * - xoops_rebuildQueryString() — parse-and-re-emit a query string for safe reflection * * They originally lived in include/cp_functions.php, but that file * unconditionally `define()`s XOOPS_CPFUNC_LOADED, which include/ @@ -309,3 +310,33 @@ function xoops_isLocalUrl($url) return $sameHost && $sameScheme && $samePort; } } + +if (!function_exists('xoops_rebuildQueryString')) { + /** + * Rebuild a query string so it is safe to reflect into a redirect + * Location header, '?' included — or return '' when there is nothing + * usable. The raw QUERY_STRING is attacker-controlled: appended verbatim + * it invites cache-poisoning and phishing parameter injection (CRLF + * itself is already blocked by PHP's header()). Instead of gating on a + * character allowlist — which reflects malformed percent-escapes + * verbatim and costs a long urlencoded xoops_redirect its whole query — + * the string is parsed with parse_str(), which mirrors how the redirect + * target itself will read it, and re-emitted with http_build_query(), so + * every reflected byte is RFC 3986-safe or a valid escape by + * construction. The length cap is header-size sanity only. + * + * @param string $queryString raw query string (e.g. $_SERVER['QUERY_STRING']) + * @return string '?' plus the rebuilt query string, or '' when empty, oversized or unparseable + */ + function xoops_rebuildQueryString($queryString) + { + $queryString = (string) $queryString; + if ('' === $queryString || strlen($queryString) > 2000) { + return ''; + } + parse_str($queryString, $params); + $rebuilt = http_build_query($params, '', '&', PHP_QUERY_RFC3986); + + return ('' === $rebuilt) ? '' : ('?' . $rebuilt); + } +} diff --git a/htdocs/language/english/user.php b/htdocs/language/english/user.php index a5a9fb748..5775a44a4 100644 --- a/htdocs/language/english/user.php +++ b/htdocs/language/english/user.php @@ -134,3 +134,5 @@ //XOOPS 2.5.11 define('_US_DESCRIPTIONMIN', 'Minimum required length: %s'); define('_US_DESCRIPTIONMAX', 'Maximum length: %s'); +// XOOPS 2.7.3 +define('_US_SURETOLOGOUT', 'Are you sure you want to log out?'); diff --git a/htdocs/modules/pm/preloads/core.php b/htdocs/modules/pm/preloads/core.php index 117fa372a..a75c49e94 100644 --- a/htdocs/modules/pm/preloads/core.php +++ b/htdocs/modules/pm/preloads/core.php @@ -29,12 +29,27 @@ */ class PmCorePreload extends XoopsPreloadItem { + /** + * The current request's query string, rebuilt for safe reflection into a + * redirect Location target, '?' included — or '' when there is nothing + * usable. One shared implementation: see xoops_rebuildQueryString() in + * include/file_safety.php for the threat model. + * + * @return string + */ + private static function filteredQueryString() + { + require_once XOOPS_ROOT_PATH . '/include/file_safety.php'; + + return xoops_rebuildQueryString($_SERVER['QUERY_STRING'] ?? ''); + } + /** * @param $args */ public static function eventCorePmliteStart($args) { - header('location: ./modules/pm/pmlite.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + header('location: ./modules/pm/pmlite.php' . self::filteredQueryString()); exit(); } @@ -43,7 +58,7 @@ public static function eventCorePmliteStart($args) */ public static function eventCoreReadpmsgStart($args) { - header('location: ./modules/pm/readpmsg.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + header('location: ./modules/pm/readpmsg.php' . self::filteredQueryString()); exit(); } @@ -52,7 +67,7 @@ public static function eventCoreReadpmsgStart($args) */ public static function eventCoreViewpmsgStart($args) { - header('location: ./modules/pm/viewpmsg.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + header('location: ./modules/pm/viewpmsg.php' . self::filteredQueryString()); exit(); } diff --git a/htdocs/modules/profile/preloads/core.php b/htdocs/modules/profile/preloads/core.php index e10712d77..9a00a8369 100644 --- a/htdocs/modules/profile/preloads/core.php +++ b/htdocs/modules/profile/preloads/core.php @@ -31,6 +31,21 @@ */ class ProfileCorePreload extends XoopsPreloadItem { + /** + * The current request's query string, rebuilt for safe reflection into a + * redirect Location target, '?' included — or '' when there is nothing + * usable. One shared implementation: see xoops_rebuildQueryString() in + * include/file_safety.php for the threat model. + * + * @return string + */ + private static function filteredQueryString() + { + require_once XOOPS_ROOT_PATH . '/include/file_safety.php'; + + return xoops_rebuildQueryString($_SERVER['QUERY_STRING'] ?? ''); + } + /** * @param $args */ @@ -44,7 +59,7 @@ public static function eventCoreUserStart($args) } $from = Request::getString('from', '', 'GET'); if ($op !== 'login' && $from !== 'profile') { - header('location: ./modules/profile/user.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + header('location: ./modules/profile/user.php' . self::filteredQueryString()); exit(); } } @@ -54,7 +69,7 @@ public static function eventCoreUserStart($args) */ public static function eventCoreEdituserStart($args) { - header('location: ./modules/profile/edituser.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + header('location: ./modules/profile/edituser.php' . self::filteredQueryString()); exit(); } @@ -74,7 +89,7 @@ public static function eventCoreLostpassStart($args) */ public static function eventCoreRegisterStart($args) { - header('location: ./modules/profile/register.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + header('location: ./modules/profile/register.php' . self::filteredQueryString()); exit(); } @@ -83,7 +98,7 @@ public static function eventCoreRegisterStart($args) */ public static function eventCoreUserinfoStart($args) { - header('location: ./modules/profile/userinfo.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + header('location: ./modules/profile/userinfo.php' . self::filteredQueryString()); exit(); } } diff --git a/htdocs/modules/profile/register.php b/htdocs/modules/profile/register.php index 69e9eb7cb..3e18c12f9 100644 --- a/htdocs/modules/profile/register.php +++ b/htdocs/modules/profile/register.php @@ -29,7 +29,13 @@ $regOp = Request::getString('op', '', 'GET'); if ($regOp !== '' && in_array($regOp, ['actv', 'activate'])) { - header('location: ./activate.php' . (empty($_SERVER['QUERY_STRING']) ? '' : '?' . $_SERVER['QUERY_STRING'])); + // Rebuild the query string before reflecting it into the Location header: + // reflected verbatim, the attacker-controlled QUERY_STRING invites + // cache-poisoning and phishing parameter injection. One shared + // implementation: see xoops_rebuildQueryString() in include/file_safety.php + // for the threat model. + require_once XOOPS_ROOT_PATH . '/include/file_safety.php'; + header('location: ./activate.php' . xoops_rebuildQueryString($_SERVER['QUERY_STRING'] ?? '')); exit(); } diff --git a/htdocs/modules/profile/user.php b/htdocs/modules/profile/user.php index 0543554ec..b174e5a30 100644 --- a/htdocs/modules/profile/user.php +++ b/htdocs/modules/profile/user.php @@ -93,6 +93,23 @@ } if ($op === 'logout') { + // A logout must present a valid session token: with none required, any + // third-party page could end the visitor's session through a bare GET + // (forced-logout CSRF). Existing tokenless links keep working — they land + // on a POST confirmation (which carries the token) instead of acting + // immediately, so the many cached theme and block templates that emit + // user.php?op=logout links need no change. + if (!$GLOBALS['xoopsSecurity']->check()) { + // Translation packs predating 2.7.3 Final lack this constant, and an + // undefined constant is a fatal Error on PHP 8 — fall back to the + // English string rather than taking down the logout page. + defined('_US_SURETOLOGOUT') || define('_US_SURETOLOGOUT', 'Are you sure you want to log out?'); + include $GLOBALS['xoops']->path('header.php'); + include __DIR__ . '/header.php'; + xoops_confirm(['op' => 'logout'], 'user.php', _US_SURETOLOGOUT); + include __DIR__ . '/footer.php'; + exit(); + } $message = ''; // Regenerate a new session id and destroy old session $GLOBALS['sess_handler']->regenerate_id(true); diff --git a/htdocs/xoops_lib/modules/protector/module_icon.php b/htdocs/xoops_lib/modules/protector/module_icon.php index 28993403d..65e32a3e3 100644 --- a/htdocs/xoops_lib/modules/protector/module_icon.php +++ b/htdocs/xoops_lib/modules/protector/module_icon.php @@ -54,7 +54,7 @@ $px = (92 - 6 * strlen($mydirname)) / 2; imagestring($im, 3, $px, 34, $mydirname, $color); imagepng($im); - imagedestroy($im); + unset($im); // frees the GdImage; imagedestroy() is deprecated in PHP 8.5 } else { readfile($icon_fullpath); } diff --git a/tests/unit/htdocs/include/RebuildQueryStringTest.php b/tests/unit/htdocs/include/RebuildQueryStringTest.php new file mode 100644 index 000000000..205f981f7 --- /dev/null +++ b/tests/unit/htdocs/include/RebuildQueryStringTest.php @@ -0,0 +1,82 @@ + */ + public static function rebuiltStrings(): array + { + return [ + 'plain pair' => ['uid=5', '?uid=5'], + 'multiple pairs' => ['send=1&to_userid=5', '?send=1&to_userid=5'], + 'token passthrough' => ['op=logout&XOOPS_TOKEN_REQUEST=abc123', '?op=logout&XOOPS_TOKEN_REQUEST=abc123'], + 'malformed escape' => ['a=%ZZ', '?a=%25ZZ'], + 'trailing percent' => ['a=%', '?a=%25'], + 'encoded crlf' => ['evil=%0d%0aSet-Cookie:x=1', '?evil=%0D%0ASet-Cookie%3Ax%3D1'], + 'markup' => ['a=