SchemaCache is prone to persistent cache corruption / drupal_get_complete_schema() is not robust against recursion

Created on 11 March 2015, about 10 years ago
Updated 13 April 2024, about 1 year ago

Problem/Motivation

SchemaCache is prone to persistent cache corruption.

protected function resolveCacheMiss($offset) {
    $complete_schema = drupal_get_complete_schema();
    $value = isset($complete_schema[$offset]) ? $complete_schema[$offset] : NULL;
    $this->storage[$offset] = $value;
    $this->persist($offset);
    return $value;
  }

If drupal_get_complete_schema() returns NULL (which is a valid case thats handled in that function) then the bad values are persisted.

This can be reproduced by calling e.g. entity_get_info('node') before DRUPAL_BOOTSTRAP_FULL and e.g. a corrupted module implements cache can lead to that. (see also 🐛 module_implements() cache can be polluted by module_invoke_all() being called (in)directly prior to full bootstrap completion RTBC )

Proposed resolution

Lets harden this for the most obvious corruption:

diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc
index 0b81dc0..e843997 100644
--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -2972,17 +2972,34 @@ class SchemaCache extends DrupalCacheArray {
   }
 
   /**
+   * Whether the cache should be persisted.
+   */
+  protected $persistable;
+
+  /**
    * Overrides DrupalCacheArray::resolveCacheMiss().
    */
   protected function resolveCacheMiss($offset) {
     $complete_schema = drupal_get_complete_schema();
     $value = isset($complete_schema[$offset]) ? $complete_schema[$offset] :  NULL;
-    $this->storage[$offset] = $value;
-    $this->persist($offset);
+    if ($this->persistable() && !empty($complete_schema)) {
+      $this->storage[$offset] = $value;
+      $this->persist($offset);
+    }
     return $value;
   }
 }
 
+  /**
+   * Whether the cache should be persisted.
+   */
+  function persistable() {
+    // Try until we have reached full bootstrap level.
+    if (empty($this->persistable)) {
+      $this->persistable = drupal_get_bootstrap_phase() == DRUPAL_BOOTSTRAP_FULL;
+    }
+    return $this->persistable;
+  }
+
 /**
  * Gets the whole database schema.
  *

Note: Do not apply this patch to a site lower than Drupal 7.33, because there drupal_get_bootstrap_phase() was just fixed.

Remaining tasks

<!-- See https://drupal.org/core-mentoring/novice-tasks for tips on identifying novice tasks. Delete or add "Novice" from the Novice? column in the table below as appropriate. Uncomment tasks as the issue advances. Update the Complete? column to indicate when they are done, and maybe reference the comment number where they were done. -->

User interface changes

API changes

🐛 Bug report
Status

Needs review

Version

7.0 ⚰️

Component
Cache 

Last updated about 16 hours ago

Created by

🇩🇪Germany Fabianx

Live updates comments and jobs are added and updated live.
  • Needs framework manager review

    It is used to alert the framework manager core committer(s) that an issue significantly impacts (or has the potential to impact) multiple subsystems or represents a significant change or addition in architecture or public APIs, and their signoff is needed (see the governance policy draft for more information). If an issue significantly impacts only one subsystem, use Needs subsystem maintainer review instead, and make sure the issue component is set to the correct subsystem.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇮🇹Italy apaderno Brescia, 🇮🇹

    The premises of this issue are that drupal_get_complete_schema() can return NULL, but for what I can see, that never happen. It can eventually return an empty array, since It even casts to an array the value returned from hook_schema().

    // Cast the result of hook_schema() to an array, as a NULL return value
    // would cause array_merge() to set the $schema variable to NULL as well.
    // That would break modules which use $schema further down the line.
    $current = (array) module_invoke($module, 'schema');
    

    drupal_get_complete_schema() should not return an empty array, with the System module that is essential for Drupal, and which defines the schema for at least 26 database tables.
    The code should not cache a not complete schema, which means waiting to cache the value returned from drupal_get_complete_schema() until the bootstrap phase is DRUPAL_BOOTSTRAP_FULL.

Production build 0.71.5 2024