Security review fixes (plugin): GeoIP reader tracking, .htaccess, data bounds, token hashing

.github/dependabot.yml

@@ -0,0 +1,16 @@
+version: 2
+updates:
+  # the vendored GeoIP reader (src/Helper/Geo): a new release of
+  # maxmind-db/reader gets a pull request; its CI run fails until
+  # `composer install && composer vendor-geoip` has been run and the result
+  # committed - see README, "GeoIP reader"
+  - package-ecosystem: composer
+    directory: /
+    schedule:
+      interval: weekly
+    labels: [dependencies, geoip-reader]
+  - package-ecosystem: github-actions
+    directory: /
+    schedule:
+      interval: weekly
+    labels: [dependencies]

.github/workflows/ci.yml

@@ -25,6 +25,14 @@ jobs:
           find . -name '*.xml' -not -path './.git/*' -print0 | xargs -0 xmllint --noout
       - name: Language files
         run: php tests/lint/check-language.php bfstop.xml PLG_SYSTEM_BFSTOP
+      - name: Composer manifest is valid
+        run: composer validate --strict
+      - name: Vendored GeoIP reader is the version locked in composer.lock
+        # src/Helper/Geo is generated by tools/vendor-geoip.php; if this fails
+        # after a dependency update, run `composer install && composer vendor-geoip`
+        run: |
+          composer install --no-interaction --no-progress
+          composer check-geoip
       - name: Release zip contains everything the manifest references
         run: |
           ./deploy.sh zip

.github/workflows/dependencies.yml

@@ -0,0 +1,43 @@
+name: Dependency check
+
+# The plugin ships a copy of the MaxMind DB reader (src/Helper/Geo, see README).
+# This weekly job reports a newer release, and security advisories against the
+# locked version, even if Dependabot is switched off.
+
+on:
+  schedule:
+    - cron: '23 5 * * 1'
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  issues: write
+
+jobs:
+  geoip-reader:
+    name: maxmind-db/reader is current and free of advisories
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7
+      - uses: shivammathur/setup-php@v2
+        with:
+          php-version: '8.4'
+          coverage: none
+      - run: composer install --no-interaction --no-progress
+      - name: Vendored copy is the locked version
+        run: composer check-geoip
+      - name: Security advisories
+        run: composer audit
+      - name: Newer release available?
+        run: |
+          composer outdated --direct --strict maxmind-db/reader
+      - name: Open an issue
+        if: failure()
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          title="Update the vendored GeoIP reader (maxmind-db/reader)"
+          if [ -z "$(gh issue list --state open --search "in:title $title" --json number --jq '.[0].number')" ]; then
+            gh issue create --title "$title" --label dependencies \
+              --body "The weekly dependency check failed: maxmind-db/reader has a newer release, a security advisory, or src/Helper/Geo differs from composer.lock. See the run: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID. To update: \`composer update maxmind-db/reader && composer vendor-geoip\`, review the diff, run the tests, release both extensions."
+          fi

.gitignore

@@ -0,0 +1,2 @@
+/vendor/
+*.zip

CHANGELOG

@@ -152,6 +152,35 @@
     are validated (no stream wrappers like phar://, a .mmdb file name)
   - Added SECURITY.md with the supported versions and how to report
     vulnerabilities
+  - Security: the bundled MaxMind DB reader (maxmind-db/reader, now 1.14.0,
+    which limits the decoding of malformed or hostile database files) is
+    tracked in composer.json/composer.lock; src/Helper/Geo is generated from
+    it (tools/vendor-geoip.php), CI verifies that, and Dependabot plus a
+    weekly job report new releases and security advisories
+  - Security: blocking through .htaccess removes blocks that ran out or were
+    lifted from the file (a timed block used to stay there for ever, and the
+    file grew with every blocked address); at most 2000 lines are kept; the
+    file is replaced atomically instead of being rewritten in place; the lock
+    file lives in Joomla's tmp directory instead of the system's
+  - Security: failed logins are purged after 4 weeks by default (set the
+    purge age to 0 for the old behaviour) and limited to 200000 rows
+  - Security: a response is delayed by 60 seconds at most, however the delay
+    settings add up, and the account throttle no longer delays an address
+    that was just blocked
+  - Security: unblock tokens are stored as hashes; links sent before the
+    update stop working (they are valid for 3 days only)
+  - Security: usernames in log entries and in mails can no longer contain
+    line breaks; array-valued request parameters ("option[]=x") no longer
+    cause a TypeError in the check of blocked requests
+  - Settings which have not been saved yet now have the values the settings
+    page shows (block after 10 attempts, ...)
+  - Component: all list settings are validated against their options, the
+    notification addresses against the email format; messages escape what was
+    typed in; the settings and log views need the core.admin permission
+  - Internal: the Joomla! APIs deprecated in 4.3-5.3 and removed in Joomla! 7
+    are no longer used (Factory::getDbo/getConfig/getSession/getLanguage/
+    getDocument/getMailer, $_db, Table::getInstance, View::get(),
+    addStyleSheet(), getCfg()); a test fails if one comes back
 
 1.5.2 (2024-02-18)
   - Note: Only component changes, no plugin changes

README

@@ -19,3 +19,27 @@ https://github.com/codeling/bfstop
 For any further questions, don't hesitate to contact me under
 info@bfstop.de
 
+GeoIP reader (third-party code, keep it up to date!)
+----------------------------------------------------
+The optional GeoIP features (country risk score, "Information for IP Address")
+use a copy of the MaxMind DB reader (maxmind-db/reader, Apache-2.0), shipped in
+src/Helper/Geo, because a Joomla! extension cannot rely on composer at install
+time. It parses a database file with untrusted-format binary data, so it has to
+be updated whenever MaxMind publishes a new release - releases of this plugin
+should never ship an outdated copy.
+
+  * Which version is shipped:  composer.lock (package maxmind-db/reader); the
+    header of src/Helper/Geo/Reader.php names it too.
+  * How it is kept current:  Dependabot and a weekly GitHub Actions job
+    (.github/workflows/dependencies.yml) report new releases and security
+    advisories; CI fails if src/Helper/Geo is not exactly the locked version.
+  * How to update:  composer update maxmind-db/reader && composer vendor-geoip,
+    review the diff, run the tests, release plugin and component.
+  * The GeoLite2/GeoIP2 *database* (.mmdb) is separate: you download it and
+    configure its path in the component's settings. MaxMind publishes new
+    versions of GeoLite2 twice a week (Tuesdays and Fridays), and the
+    addresses of the internet move around, so an old file gets less and less
+    accurate - it only feeds the risk score and the address information, it is
+    not a security boundary. Update it regularly (geoipupdate can do that from
+    cron), and check the licence terms of the database you use: they may
+    require you to keep it current.

SECURITY.md

@@ -35,3 +35,13 @@ Settings are changed by users with the `core.admin` permission for
 `com_bfstop`. They can, among other things, choose the directory in which the
 plugin writes its `.htaccess` file, so grant that permission only to people
 you trust with the web server's configuration for your site.
+
+## Third-party code
+
+The plugin ships a copy of MaxMind's `maxmind-db/reader` in `src/Helper/Geo`
+(the exact version is pinned in `composer.lock`). A weekly CI job and
+Dependabot report new releases and security advisories, and CI fails if the
+copy deviates from the pinned version. Releases are only made with the latest
+reader; see the "GeoIP reader" section of the README for how to update it. If
+you notice that the shipped version is behind, please report it like any other
+vulnerability.

composer.json

@@ -0,0 +1,20 @@
+{
+    "name": "codeling/bfstop",
+    "description": "Brute Force Stop for Joomla! - this file only tracks the third-party code vendored into the plugin package (src/Helper/Geo), see README",
+    "type": "project",
+    "license": "GPL-3.0-or-later",
+    "require": {
+        "php": ">=8.1",
+        "maxmind-db/reader": "^1.14.0"
+    },
+    "config": {
+        "sort-packages": true,
+        "platform": {
+            "php": "8.1.0"
+        }
+    },
+    "scripts": {
+        "vendor-geoip": "@php tools/vendor-geoip.php",
+        "check-geoip": "@php tools/vendor-geoip.php --check"
+    }
+}