Repository navigation
Security review fixes (component): settings validation, escaping, permissions, token hashes - #18
Merged
Merged
Conversation
The plugin stores only a hash of each unblock token now (DatabaseHelper::hashToken()); the unblock page hashes the token of the link the same way. Needs the matching bfstop change (branch security-review-fixes). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LhhhdjhErC9ZiaS4HyRjsh
…s and log - Every list/integer setting is validated against its options (Joomla only does that with validate="options"), the notification addresses against the email format; the 'maxBlocksBefore' option 6 had the value 5. - Messages built from what was typed into a form are escaped: Joomla shows them as HTML. - The settings and log views need core.admin, not just core.manage. - The allow list removal casts the ids itself and reports what happened; the unblock helper no longer reads an undefined result after an exception; the unblock page works while the plugin is disabled; the failed login list falls back to its own sort column. Tests are in the bfstop repository (tests/Integration/ComponentTest.php). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LhhhdjhErC9ZiaS4HyRjsh
Models use their own database (getDatabase()) instead of Factory::getDbo() and the deprecated $_db property, so they also work with the database a caller injects; the controller and helpers get the DatabaseInterface from the container; SettingsModel creates the extension table directly instead of via Table::getInstance(). All three are deprecated and gone in Joomla 7. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LhhhdjhErC9ZiaS4HyRjsh
- $app->input -> getInput(); CMSApplication::getCfg() -> get() - Views ask their model (->getModel()->getItems() ...) instead of the deprecated View::get() (deprecated in Joomla 5.3) - Stylesheets of the edit views are registered with the WebAssetManager instead of Document::addStyleSheet(); the path is relative to the site root (a leading slash would give a protocol-relative URL) - Factory::getLanguage()/getSession() -> the application's getLanguage()/ getSession() The test for the deprecated calls is in the bfstop repository (tests/Unit/DeprecatedApiTest.php). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LhhhdjhErC9ZiaS4HyRjsh
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
2 participants
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
Component side of the security review fixes; the plugin side is in codeling/bfstop (branch
security-review-fixes, which also holds all tests - they cover both extensions).list/integersetting hasvalidate="options"(Joomla does not check submitted values against the options otherwise); notification addresses are checked (emaillistrule); themaxBlocksBeforeoption "6" had the value 5.IpValidateHelper) are escaped - Joomla renders messages as HTML.core.admin(wascore.manage).UnblockHelper::unblockDBno longer reads an undefined variable after an exception; unblock page works while the plugin is disabled; failed login list fallback ordering column.Existing saved values that do not fit a setting's options (e.g. a delay not a multiple of 5) must be re-selected when the settings are next saved.
Testing
Run from the bfstop repository with both branches checked out: 322 tests passing on Joomla 5.4.8 / PostgreSQL 16; the new component tests fail against the unfixed code.
check-language.phphas no errors (pre-existing missing-translation warnings only). Not run: MySQL/MariaDB, Joomla 6.🤖 Generated with Claude Code
https://claude.ai/code/session_01LhhhdjhErC9ZiaS4HyRjsh
Generated by Claude Code