Problem/Motivation
This module has a rate-limiting / flood-control vulnerability in the PasswordReset DataProducer.
Summary
PasswordReset DataProducer (src/Plugin/GraphQL/DataProducer/User/PasswordReset.php) delegates to Drupal core's UserAuthenticationController::resetPassword() with a comment stating the client IP is preserved "for flood control." However, UserAuthenticationController::resetPassword() does not enforce flood control - only login() calls floodControl(). The HTML form equivalent (UserPasswordForm) does implement flood protection, but neither the API controller nor the
DataProducer does.
This allows unlimited password reset emails to be sent to any user account via the GraphQL endpoint.
It was decided by the Drupal security team that this can be posted as public security improvement and does not need a private security issue.
Steps to reproduce
- Install Drupal 11 with
drupal/graphql:5.x-dev - Create a custom schema extension that wires the
password_resetdata producer to apasswordResetmutation - Run repeated requests:
for i in $(seq 1 100); do curl -s -X POST https://your-site.com/graphql -H 'Content-Type: application/json' -d '{"query":"mutation { passwordReset(email: "victim@example.com") { ... on PasswordResetSuccess { message } } }"}' done - All 100 requests succeed - no rate limiting is applied
Impact
- Inbox flooding of any user account
- Email provider quota exhaustion (SendGrid, Mailgun, etc.)
- The misleading comment in the code ("needed for flood control") prevents discovery of the issue
Proposed fix
Inject UserFloodControlInterface and check flood limits before delegating to the controller, using Drupal's user.flood configuration.
Note
A graphql_test_auth module with GraphQL extension that uses PasswordReset DataProducer is attached
| Comment | File | Size | Author |
|---|---|---|---|
| graphql_test_auth.zip | 3.29 KB | tibezh |
Issue fork graphql-3587790
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:
Comments
Comment #3
tibezh commentedMerge request has been added
Needs review
Comment #4
klausiComment #5
klausiSame in Drupal core, opened #3587794: Missing flood control in password reset routes for that.
Comment #7
kingdutchPushed a merge to get the tests moving.
This needs work. Klausi's feedback was marked as resolved but from what I can tell it was not addressed since we're still telling attackers why their flood was blocked:
Beyond that it looks good.
Comment #8
klausiAfter thinking about it I think it is fine to show a specific error message to users (and attackers) as we have it now. I don't see the big security benefit of hiding that and it creates confusion in testing scenarios when it does not send emails.
I also think it is fine to re-use the user flood control settings, good enough as we documented in the comments.
Remaining steps:
* Fix phpunit tests fails
* Add new test case to trigger the flood control
Comment #9
tibezh commentedTests are fixed, added a
testFloodControlBlocksRepeatedAttempts()test case.Comment #10
klausiThanks, merge request looks good!
I added it to the merge train, will be pushed soon.