Skip to content

[3.0] Only enforce bans that actually exist - #9310

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:fix/enforce-bans-missing-guard
Open

[3.0] Only enforce bans that actually exist#9310
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:fix/enforce-bans-missing-guard

Conversation

@albertlast

Copy link
Copy Markdown
Collaborator

Note

This change was produced by an LLM. The code, the commit message and this
description were all written by Claude (Anthropic), driven by @albertlast. It has
not yet had human code review.

Everything stated below was verified by actually running it against a fresh
PostgreSQL install of this branch, rather than only reasoned about. Even so,
please review it as untrusted work: the diagnosis may be right while the fix is
not what SMF would prefer stylistically or architecturally.

Description

User::enforceBans() walks the four ban restrictions and reads $bans[$restriction]
without checking whether that restriction is present. The cannot_access case then
sets $should_kick = true unconditionally:

foreach ($restrictions as $restriction) {
    $ban = $bans[$restriction];          // no existence check

    switch ($restriction) {
        case 'cannot_access':
            $should_kick = true;         // always, ban or no ban

So on a forum with no bans at all, every non-admin is treated as fully banned. By
the time execution reaches Logging::logBan($ban['ids']), $ban is null:

Undefined array key "cannot_access" in Sources/User.php:2106
Trying to access array offset on null in Sources/User.php:2168
Logging::logBan(): Argument #1 ($ban_ids) must be of type array, null given

The practical effect on a clean 3.0 install is that every page returns an error for
guests. There is no way out from inside the forum either: guests cannot reach the
login page, so you cannot sign in as an admin to change anything.

SMF 2.1 guarded this with if (!empty($_SESSION['ban'][$restriction])). The guard
was lost when ban handling moved into this loop in e3cb03d. This restores it, so
restrictions that were never recorded are skipped.

How this was verified

  • Fresh 3.0 install on PostgreSQL 17, PHP 8.4.
  • Before: guest requests to index.php return HTTP 500.
  • After: HTTP 200, board list and login link render, error log stays empty
    across repeated requests.
  • Separately inserted a real cannot_access ban on the requesting IP and confirmed
    enforcement still fires, so this does not simply disable banning.

Of the issues found in this area, this is the one that makes the forum unusable, so
it may be worth handling independently of the related PRs below.

Issues References (Fixes|Related|Closes)

  1. No existing issue found for this.

User::enforceBans() looped over the four ban restrictions and read
$bans[$restriction] without checking whether that restriction was
present. The 'cannot_access' case then set $should_kick unconditionally,
so every non-admin was treated as fully banned, and $ban was null by the
time it reached Logging::logBan():

  Undefined array key "cannot_access" in Sources/User.php:2106
  Trying to access array offset on null in Sources/User.php:2168
  Logging::logBan(): Argument #1 ($ban_ids) must be of type array,
  null given

The practical effect on a clean install is that every page returns an
error for guests. Because guests cannot reach the login page either,
there is no way to sign in as an admin to escape it.

SMF 2.1 guarded this with !empty($_SESSION['ban'][$restriction]); the
guard was lost when the ban handling moved into this loop. Restores it,
so restrictions that were never recorded are skipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant