[#48141] - [5.4] [AI] Use Crypt::timingSafeCompare for the CSRF header token check in Session::checkToken
- Closed
- 24 Jul 2026
- Medium
- Build: 5.4-dev
- # 48141
- Diff
- Nexory:fix/session-checktoken-timing-safe
User tests: Successful: Unsuccessful:
Pull Request resolves # .
- I read the Generative AI policy and my contribution is either not created with the help of AI or is compatible with the policy and GNU/GPL 2 or later.
This change was prepared with AI assistance and is compatible with the policy: it is a single-line change that routes an existing comparison through Joomla's own GPL-licensed Crypt::timingSafeCompare() helper, is human-reviewed and tested, and introduces no third-party or non-GPL code.
Summary of Changes
Session::checkToken() compares the CSRF form token against the incoming X-CSRF-TOKEN request header with a plain === string comparison. This routes that comparison through Crypt::timingSafeCompare(), Joomla's own constant-time helper (already used in MD5Handler and the com_users BackupcodesModel), so the token is not compared in a way whose duration depends on how many leading bytes match.
Behavior is unchanged for both matching and non-matching tokens; only the comparison mechanism changes. This is in line with the ongoing migration of secret comparisons to constant-time checks (e.g. #8353 / #8401, and the open #48056 for TOTP verification).
The query/form-field token path below the header check is intentionally left untouched: it checks for the presence of a request parameter named after the token, not a value comparison, so it is not affected by this change.
Testing Instructions
- Make an authenticated request (e.g. an AJAX call) that sends the correct session CSRF token in the
X-CSRF-TOKENheader. - Repeat the request with an incorrect
X-CSRF-TOKENheader value.
Actual result BEFORE applying this Pull Request
Step 1 is accepted, step 2 is rejected. The header CSRF token is validated with a non-constant-time === string comparison.
Expected result AFTER applying this Pull Request
Step 1 is accepted, step 2 is rejected (no functional change). The header CSRF token is validated with Crypt::timingSafeCompare(), matching how Joomla compares other secrets.
Link to documentations
Please select:
-
Documentation link for guide.joomla.org:
-
No documentation changes for guide.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
PR-5.4-dev
|
||
@Nexory Please use the https://github.com/joomla/joomla-cms/blob/5.4-dev/.github/PULL_REQUEST_TEMPLATE.md for your PR description. Your description is missing the acknowledgement of the Generative AI policy.
Sorry about that, will fix it asap
| Title |
|
||||||
tecpromotion
- | Title |
|
||||||
Please read the description of the Crypt::timingSafeCompare() method.
Thanks - I did read it. The only caveat in the docblock is "Length will leak", which doesn't apply here: getFormToken() returns a fixed-length hash, so no length information is exposed. The intent is only to route the X-CSRF-TOKEN header check through the same constant-time helper that the token API-auth plugin and the com_users backup codes already use, for consistency rather than to fix an exploitable bug. If you consider a timing vector on the session-bound token out of scope, I'm happy to close - your call.
I mean, the mothod is deprecated and should not be used anymore.
Ah, understood - thanks, I didn't realize the method was deprecated. That removes the reason for the change as written, and the timing vector here is marginal anyway, so I'll close this.
One small thing in case it helps: the docblock in libraries/src/Crypt/Crypt.php doesn't carry an @deprecated tag (neither in 5.4-dev nor 6.0-dev), so it isn't obvious from the source that the method shouldn't be used - adding one would make that clearer for the next person reading it. Thanks both for taking a look.
Ah, understood - thanks, I didn't realize the method was deprecated. That removes the reason for the change as written, and the timing vector here is marginal anyway, so I'll close this.
One small thing in case it helps: the docblock in libraries/src/Crypt/Crypt.php doesn't carry an @deprecated tag (neither in 5.4-dev nor 6.0-dev), so it isn't obvious from the source that the method shouldn't be used - adding one would make that clearer for the next person reading it. Thanks both for taking a look.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2026-07-24 20:34:24 |
| Closed_By | ⇒ | Nexory |
@Nexory Please use the https://github.com/joomla/joomla-cms/blob/5.4-dev/.github/PULL_REQUEST_TEMPLATE.md for your PR description. Your description is missing the acknowledgement of the Generative AI policy.