Use constant-time comparison for the backend secret key validation#41044
Open
Nexory wants to merge 1 commit into
Open
Use constant-time comparison for the backend secret key validation#41044Nexory wants to merge 1 commit into
Nexory wants to merge 1 commit into
Conversation
|
Hi @Nexory. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
Author
|
@magento run all tests |
Author
|
@magento run Magento Health Index |
Route the admin secret_key check in BackendValidator through hash_equals() instead of a plain === comparison, consistent with existing constant-time secret/hash comparisons in core (e.g. Sales\Helper\Guest and Paypal ReturnUrl).
Nexory
force-pushed
the
hardening/backend-secretkey-constant-time
branch
from
July 23, 2026 23:42
f50f196 to
279ad1b
Compare
Author
|
@magento run Static Tests,Unit Tests,Integration Tests,Semantic Version Checker,Magento Health Index |
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
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.
Description (*)
BackendValidator::validateRequest()validates the adminsecret_key(the anti-CSRF key embedded in backend URLs) with a plain===string comparison:This changes it to
hash_equals()so the admin secret key is compared in constant time, consistent with how other secret and hash values are compared across the codebase, for example:Magento\Sales\Helper\Guestcompares the order protect code withhash_equals((string)$order->getProtectCode(), $protectCode).Magento\Paypal\Controller\Payflow\ReturnUrlcompares the request hash withhash_equals(...).AbstractAction::_validateSecretKey()and the POST/form-key path (FormKeyValidator) compare the same class of value viaSecurity::compareStrings(), which is itself ahash_equals()wrapper.hash_equals()is used directly (rather than theSecurity::compareStrings()wrapper) to keep this file free of a new static-method call, matching the rawhash_equals()usage already present in core.It is a behavior-preserving change: the equality result is identical for matching, non-matching, and empty-secret cases. No new test is added because there is no behavioral change; existing backend request-validation coverage continues to pass.
Related Pull Requests
#40923 applies constant-time comparison to a different secret (the storefront custom-option download key, a different file); no overlap.
Fixed Issues (if relevant)
None.
Manual testing scenarios (*)
CsrfAwareActionInterfaceand does not extendAbstractAction(so the fallback secret-key path runs) with the correctkey=<secret_key>URL parameter. The request is accepted, as before.keyvalue. The request is rejected with "Invalid security or form key. Please refresh the page.", as before.There is no functional change; both scenarios behave exactly as they did prior to this PR, only the comparison is now constant-time.
Contribution checklist (*)