Problem/Motivation
If an attacker gets access to error logs and CsrfTokenGenerator::validate() throws an exception, it could leak valid CSRF tokens in the stack trace:
RuntimeException:
Missing $settings['hash_salt'] in settings.php.
at core/lib/Drupal/Core/Site/Settings.php:173
at Drupal\Core\Site\Settings::getHashSalt()
(core/lib/Drupal/Core/Access/CsrfTokenGenerator.php:115)
at Drupal\Core\Access\CsrfTokenGenerator->computeToken('yZyPqDjxGaPr_PKQ_u5GrSzhZ6cxTX8yw_w6RSFdsLs', '')
(core/lib/Drupal/Core/Access/CsrfTokenGenerator.php:89)
at Drupal\Core\Access\CsrfTokenGenerator->validate('valid_token')
(modules/custom/test/test.module:50)
Steps to reproduce
Very crude way to trigger an exception;
ini_set('zend.exception_ignore_args', FALSE);
new Settings([]);
\Drupal::csrfToken()->validate('valid_token');
Proposed resolution
Add #[SensitiveParameter] to the $token argument.
Maybe the $seed argument in computeToken() should also be considered sensitive.
(I don't think you can do anything dangerous with only the seed, you would also need the private and the hash salt.)
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3582512
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3582512-add-sensitiveparameter-attribute
changes, plain diff MR !15313
Comments
Comment #2
sourav_paulWorking on this...
Comment #4
sourav_paulComment #5
phenaproximaI don’t see a problem here.
Comment #8
amateescu commentedMakes sense to me as well. And I agree that
$seedfromcomputeToken()should not be considered sensitive because it's useless without the private key and hash salt.Committed and pushed b528aa6577d to main and a79fcb2f690 to 11.x. Thanks!
Comment #11
amateescu commentedComment #14
amateescu commentedDiscussed this issue with the release managers, and decided to backport it all the way back to 10.6.x.