Background information

This was originally reported as a private security issue, but has been approved for handling in the public queue by the Drupal Security Team.

Under PSA-2023-07-12, Drupal explicitly states that issues involving denial of service, scalability under traffic, or unhandled exceptions are routinely treated as standard bugs to be fixed in the public queue, rather than being kept private.

Problem/Motivation

When the database query responsible for counting recent tracking events encounters an exception—such as during database connection exhaustion, lock-wait timeouts, or aborted transactions—the error handling mechanism fails unsafe.

  • The Catch Block Flaw: The query is wrapped in a try/catch block. If an exception occurs, the catch block triggers a check to verify if the tracking table exists. Because this check returns a positive confirmation if the table is already present, the method defaults to granting clearance.
  • Operational Impact: Instead of restricting actions when a database error occurs, the system mistakenly treats the failure as an indication that operational limits have not been reached. As a result, rate-limiting throttles are silently bypassed for that specific request.
  • Logging Failures: Because the system assumes the request is valid, no error or bypass activity is written to the system logs. Furthermore, a related issue in the registration tracking component causes an uncaught error (resulting in an HTTP 500 response) under the same conditions, which similarly fails to log the event.

Proposed resolution

This behavior undermines the reliability of rate-limiting controls when the underlying database is under heavy load. It is recommended to apply the latest updates to ensure that database query failures are handled securely and that limits remain strictly enforced.

In core/lib/Drupal/Core/Flood/DatabaseBackend.php::isAllowed(), only return TRUE when the table genuinely did not exist. If the table exists but the query failed, fail closed (return FALSE / deny). Apply the same to register() so a failed retry is logged, not an uncaught fatal. (The existing catchException() helper already uses the correct “table exists → propagate” pattern.)

For example,

 catch (\Throwable) {
      if ($this->connection->schema()->tableExists(static::TABLE_NAME)) {
        return FALSE;
      }
      $this->ensureTableExists();
      return TRUE;

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3606380

Command icon 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

mingsong created an issue. See original summary.

deruina made their first commit to this issue’s fork.

deruina’s picture

Status: Active » Needs review

Opened an MR. isAllowed() now uses the existing catchException() helper, so if the table exists and the query fails the exception is thrown instead of returning TRUE. I went with rethrowing rather than returning FALSE because it matches clear() and garbageCollection() and the error gets logged. I left register() alone since it already throws on a failed retry.

AI-Generated: Yes (drafted with Claude)