[3.0] Misc fixes by Sesquipedalian · Pull Request #9302 · SimpleMachines/SMF

Sesquipedalian · 2026-07-18T20:23:05Z

Security fix: uses hash_equals() in SMF\Maintenance\Maintenance::loginAdmin() and SMF\Maintenance\Maintenance::loginWithDatabasePassword() methods.
Logs correct info about who ran the upgrader. Previously, we were doing some silly things with loading and unloading User::$me. Not only was that unnecessary, and not only could it produce inaccurate info in the logs under certain circumstances, but it also further complicated the problems with getting the admin's login cookie right by the time the upgrade was complete.
Moves SMF\Actions\Login2::validatePasswordFlood() to SMF\Security::validatePasswordFlood().
Implements SMF\Security::checkPassword().
Fixes bugs with handling login security tokens in the upgrader.
Point 5 ultimately led to replacing Maintenance::$context with just plain old Utils::$context. Upon closer inspection, I realized that there was neither any need nor any benefit to having a separate version of $context; it just caused headaches when using other code, such as SecurityToken::create(), that is already designed to work with Utils::$context.

Sesquipedalian · 2026-07-19T03:13:02Z

Okay, I think I am done messing with this PR.

@sbulen, would you care to test whether it solves the problem of not being logged after the upgrader exits?

sbulen · 2026-07-23T18:30:29Z

2.1 => 3.0

Fatal error: Uncaught Error: Cannot access uninitialized non-nullable property SMF\User::$username by reference in D:\wamp64\www\van2130\Sources\User.php:807 Stack trace: # 0 [internal function]: SMF\User->$member_name::get() # 1 D:\wamp64\www\van2130\Sources\Utils.php(1539): json_encode(Array, 0, 512) # 2 D:\wamp64\www\van2130\Sources\Services\ErrorHandlerService.php(286): SMF\Utils::jsonEncode(Array) # 3 D:\wamp64\www\van2130\Sources\Services\ErrorHandlerService.php(185): SMF\Services\ErrorHandlerService->log('Cannot access u...', 'general', 'D:/wamp64/www/v...', 807, Array) # 4 D:\wamp64\www\van2130\Sources\ErrorHandler.php(104): SMF\Services\ErrorHandlerService->catch(Object(Error)) # 5 [internal function]: SMF\ErrorHandler::catch(Object(Error)) # 6 {main} thrown in D:\wamp64\www\van2130\Sources\User.php on line 807

PHP, MySQL, Apache logs: nothing
Upgrader log: nothing

Sesquipedalian · 2026-07-24T05:46:49Z

Well, phooey. I must have broken something in my tinkering after I first pushed. I suppose that's what I get for trying to code while sleepy.

Sesquipedalian · 2026-08-05T04:48:13Z

@sbulen:

I've made changes to this PR. Have they fixed the error about uninitialized properties?
If so, does this PR solve the problem of not being logged after the upgrader exits?

sbulen · 2026-08-08T17:15:55Z

2.1 => 3.0

Getting a different WSOD:

Fatal error: Uncaught Error: Call to a member function allowedTo() on array in D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php:661 Stack trace: 0 D:\wamp64\www\van2130\Sources\Maintenance\Tools\Upgrade.php(819): SMF\Maintenance\Maintenance::loginAdmin('admin_shawnb', Object(SensitiveParameterValue), false) 1 D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php(294): SMF\Maintenance\Tools\Upgrade->welcomeLogin() 2 D:\wamp64\www\van2130\upgrade.php(24): SMF\Maintenance\Maintenance->execute(2) 3 {main} thrown in D:\wamp64\www\van2130\Sources\Maintenance\Maintenance.php on line 661

PHP error log reflects the above. MySQL & Apache logs clear.

Upgrade.log:

2026-08-08T17:11:08+00:00
Step 1: Login
Making sure SMF can write to "D:\wamp64\www\van2130\Settings.php"... done.
Making sure SMF can write to "D:\wamp64\www\van2130\Settings_bak.php"... done.
Making sure SMF can write to "D:\wamp64\www\van2130/custom_avatar"... done.
Making sure SMF can write to "D:/wamp64/www/van2130/cache/db_last_error.php"... done.

Sesquipedalian · 2026-08-11T21:23:57Z

That should be fixed now. The cause was just a silly mistake on my part. If you could test again, @sbulen, that would be lovely!

sbulen · 2026-08-15T18:26:35Z

Note this branch currently has merge conflicts.

sbulen · 2026-08-22T17:32:07Z

For a fun romp down memory lane...
#6942 (comment)

sbulen · 2026-08-22T18:07:33Z

Let me know if you want me to retest now, or if you want to think about the attachment directory...

sbulen · 2026-08-22T20:08:26Z

Maybe a 2.1 cleanup task that ensures it's ready for the json conversion?

Sesquipedalian · 2026-08-22T23:43:34Z

There, that should fix the attachments stuff.

The AttachmentDirectory migration step is now idempotent (meaning, running the step again won't change anything) and can handle every possible format that the value might have been in before.
The RemoveTempAttachments task uses much more robust logic when figuring out the paths to the attachment directories.

Test away, @sbulen!

sbulen · 2026-08-24T22:18:49Z

2.1 => 3.0:

Upgrader log (bottom):

+++ Adding new settings... done.
+++ Removing karma... skipped.
+++ Fixing dates... done.
+++ Creating login history table... done.
+++ Converting collapsed categories... skipped.
+++ Fixing attachment directory setting... failed with error: "Class "SMF\Maintenance\Migration\v2_1\Maintenance" not found"

This looks like something simple - I'll hold off repeating the other tests for the moment.

Sesquipedalian · 2026-08-25T02:20:32Z

Derp. Yeah, looks like I forgot a use statement. I'll fix it when I'm back at a computer.

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Signed-off-by: Jon Stovell <jonstovell@gmail.com> # Conflicts: # Sources/Maintenance/Tools/Install.php

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Can be used to allow us to check a password without automatically updating it. This matters for the upgrader, where we don't want to write to the members table before it is ready to receive new data. Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Is now idempotent (meaning, running the step again won't change anything) and can handle every possible format that the value might have been in before. Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Sesquipedalian · 2026-08-30T02:00:58Z

Fixed now, @sbulen. Ready for a retest whenever you are able.

Sesquipedalian added Upgrader Login/Sessions labels Jul 18, 2026

Sesquipedalian force-pushed the 3.0/upgrader_login branch 4 times, most recently from cbac018 to a73718e Compare July 18, 2026 21:42

jdarwood007 added this to the 3.0 Alpha 5 milestone Jul 18, 2026

Sesquipedalian force-pushed the 3.0/upgrader_login branch from a73718e to dd318d6 Compare July 19, 2026 03:07

Sesquipedalian force-pushed the 3.0/upgrader_login branch 10 times, most recently from 8be3518 to 1f2c789 Compare July 19, 2026 19:10

This was referenced Jul 28, 2026

[3.0] Read cached bans from the right place in the session #9313

Merged

[3.0] Load the current user before finalize() records dates #9316

Merged

Sesquipedalian force-pushed the 3.0/upgrader_login branch from 1f2c789 to b868cf2 Compare August 2, 2026 20:50

Sesquipedalian force-pushed the 3.0/upgrader_login branch from b868cf2 to a5aef7c Compare August 11, 2026 21:22

albertlast mentioned this pull request Aug 14, 2026

[3.0]: Upgrading from 2.1 stops in DropTimeOffset, and TFA logins and old background tasks break afterwards #9521

Open

github-actions Bot added Background tasks Scheduled Tasks labels Aug 22, 2026

Sesquipedalian added 20 commits August 29, 2026 19:40

Uses hash_equals() in SMF\Maintenance\Maintenance::login* methods

4baa3a7

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Logs correct info about who ran the upgrader

812d127

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Moves validatePasswordFlood() from SMF\Actions\Login2 to SMF\Security

6ca8fff

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Implements SMF\Security::checkPassword()

f511254

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Correctly handles creating login tokens in upgrader

03c7ecc

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Removes Maintenance::$context in favour of plain old Utils::$context

9527be3

Signed-off-by: Jon Stovell <jonstovell@gmail.com> # Conflicts: # Sources/Maintenance/Tools/Install.php

Uses correct key in DropTimeOffset migration

c3092f7

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Gives CacheApi::$enable a default of 0

54e1752

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Adds #[\SensitiveParameter] attribute to password processing methods

bf17f45

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Prevents timing attacks against Security::checkPasswordFallbacks()

b44c6ea

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Ensures $offset is a string in DropTimeOffset::execute()

e55ef6a

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Avoids permission errors when karma data was retained during upgrade

9d6fda6

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Improves backward compatibility support for permissions

fb114a6

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Adds additional guards against errors about unknown permissions

a6afde7

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Ensures guest profile has been loaded in User::initializeGuest()

5a40467

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Improves migration step for updating the attachment directory settings

14cd18e

Is now idempotent (meaning, running the step again won't change anything) and can handle every possible format that the value might have been in before. Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Runs AttachmentDirectory before LegacyAttachments during upgrade

91a3f63

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Modernizes and tides code in LegacyAttachments migration step

cc04a3f

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Handles attachmentUploadDir more robustly in RemoveTempAttachments task

c385188

Signed-off-by: Jon Stovell <jonstovell@gmail.com>

Sesquipedalian force-pushed the 3.0/upgrader_login branch from d8a35f3 to c385188 Compare August 30, 2026 01:58

Uh oh!

Conversation

Sesquipedalian commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sesquipedalian commented Jul 19, 2026

Uh oh!

sbulen commented Jul 23, 2026

2.1 => 3.0

Uh oh!

Sesquipedalian commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sesquipedalian commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbulen commented Aug 8, 2026

2.1 => 3.0

Uh oh!

Sesquipedalian commented Aug 11, 2026

Uh oh!

sbulen commented Aug 15, 2026

Uh oh!

sbulen commented Aug 22, 2026

Uh oh!

sbulen commented Aug 22, 2026

Uh oh!

sbulen commented Aug 22, 2026

Uh oh!

Sesquipedalian commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbulen commented Aug 24, 2026

2.1 => 3.0:

Uh oh!

Sesquipedalian commented Aug 25, 2026

Uh oh!

Sesquipedalian commented Aug 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Sesquipedalian commented Jul 18, 2026 •

edited

Loading

Sesquipedalian commented Jul 24, 2026 •

edited

Loading

Sesquipedalian commented Aug 5, 2026 •

edited

Loading

Sesquipedalian commented Aug 22, 2026 •

edited

Loading