#396224 - SA-CORE-2009-03 - Disallow nulls and slashes from file names in theme.

2 files changed, 109 insertions, 29 deletions

diff --git a/includes/theme.inc b/includes/theme.inc
index 018afc494..80f4c9003 100644
--- a/includes/theme.inc
+++ b/includes/theme.inc
@@ -710,11 +710,16 @@ function theme() {
 function drupal_discover_template($paths, $suggestions, $extension = '.tpl.php') {
   global $theme_engine;
 
+  // Remove slashes or null to prevent files from being included from
+  // an unexpected location (especially on Windows servers).
+  $extension = str_replace(array("/", "\\", "\0"), '', $extension);
+
   // Loop through all paths and suggestions in FIFO order.
   $suggestions = array_reverse($suggestions);
   $paths = array_reverse($paths);
   foreach ($suggestions as $suggestion) {
     if (!empty($suggestion)) {
+      $suggestion = str_replace(array("/", "\\", "\0"), '', $suggestion);
       foreach ($paths as $path) {
         if (file_exists($file = $path . '/' . $suggestion . $extension)) {
           return $file;
@@ -1900,6 +1905,51 @@ function template_preprocess_page(&$variables) {
   // Add a class that tells us whether the page is viewed by an authenticated user or not.
   $body_classes[] = $variables['logged_in'] ? 'logged-in' : 'not-logged-in';
 
+  // If on an individual node page, add the node type to body classes.
+  if (isset($variables['node']) && $variables['node']->type) {
+    $body_classes[] = 'node-type-' . form_clean_id($variables['node']->type);
+  }
+  // Add information about the number of sidebars.
+  if ($variables['layout'] == 'both') {
+    $body_classes[] = 'two-sidebars';
+  }
+  elseif ($variables['layout'] == 'none') {
+    $body_classes[] = 'no-sidebars';
+  }
+  else {
+    $body_classes[] = 'one-sidebar sidebar-' . $variables['layout'];
+  }
+
+  // Populate the page template suggestions.
+  if ($suggestions = template_page_suggestions(arg())) {
+    $variables['template_files'] = $suggestions;
+    foreach ($suggestions as $suggestion) {
+      if ($suggestion != 'page-front') {
+        // Add current suggestion to page classes to make it possible to theme the page 
+        // depending on the current page type (e.g. node, admin, user, etc.) as well as
+        // more specific data like node-12 or node-edit. To avoid illegal characters in 
+        // the class, we're removing everything disallowed. We are not using 'a-z' as
+        //  that might leave in certain international characters (e.g. German umlauts).
+        $body_classes[] = preg_replace('![^abcdefghijklmnopqrstuvwxyz0-9-_]+!s', '', form_clean_id(drupal_strtolower($suggestion)));
+      }
+    }
+  }
+  // Implode with spaces.
+  $variables['body_classes'] = implode(' ', $body_classes);
+
+}
+
+/**
+ * Generate an array of page template suggestions.
+ *
+ * @param $args
+ *   An array of path arguments, such as from function arg().
+ *
+ * @return
+ *   An array of suggested template files.
+ */
+function template_page_suggestions($args) {
+
   // Build a list of suggested template files and body classes in order of 
   // specificity. One suggestion is made for every element of the current path,
   // though numeric elements are not carried to subsequent suggestions. For 
@@ -1910,46 +1960,22 @@ function template_preprocess_page(&$variables) {
   // page-node-1.tpl.php     page-node-1
   // page-node.tpl.php       page-node
   // page.tpl.php
-  $i = 0;
+
   $suggestion = 'page';
   $suggestions = array();
-  while ($arg = arg($i++)) {
+  foreach ($args as $arg) {
+    // Remove slashes or null per SA-CORE-2009-003.
+    $arg = str_replace(array("/", "\\", "\0"), '', $arg);
     $suggestions[] = $suggestion . '-' . $arg;
     if (!is_numeric($arg)) {
       $suggestion .= '-' . $arg;
     }
-    if ($suggestion != 'page') {
-      // Add current suggestion to page classes to make it possible to theme the page 
-      // depending on the current page type (e.g. node, admin, user, etc.) as well as
-      // more specific data like node-12 or node-edit. To avoid illegal characters in 
-      // the class, we're removing everything disallowed. We are not using 'a-z' as
-      //  that might leave in certain international characters (e.g. German umlauts).
-      $body_classes[] = preg_replace('![^abcdefghijklmnopqrstuvwxyz0-9-_]+!s', '', form_clean_id(drupal_strtolower($suggestion)));
-    }
   }
   if (drupal_is_front_page()) {
     $suggestions[] = 'page-front';
   }
-  if ($suggestions) {
-    $variables['template_files'] = $suggestions;
-  }
 
-  // If on an individual node page, add the node type to body classes.
-  if (isset($variables['node']) && $variables['node']->type) {
-    $body_classes[] = 'node-type-' . form_clean_id($variables['node']->type);
-  }
-  // Add information about the number of sidebars.
-  if ($variables['layout'] == 'both') {
-    $body_classes[] = 'two-sidebars';
-  }
-  elseif ($variables['layout'] == 'none') {
-    $body_classes[] = 'no-sidebars';
-  }
-  else {
-    $body_classes[] = 'one-sidebar sidebar-' . $variables['layout'];
-  }
-  // Implode with spaces.
-  $variables['body_classes'] = implode(' ', $body_classes);
+  return $suggestions;
 }
 
 /**
diff --git a/modules/simpletest/tests/theme.test b/modules/simpletest/tests/theme.test
new file mode 100644
index 000000000..f63b51571
--- /dev/null
+++ b/modules/simpletest/tests/theme.test
@@ -0,0 +1,54 @@
+<?php
+// $Id$
+
+/**
+ * @file
+ * Tests for the theme API.
+ */
+
+/**
+ * Unit tests for the theme API.
+ */
+class TemplateUnitTest extends DrupalWebTestCase {
+  function getInfo() {
+    return array(
+      'name' => t('Theme API'),
+      'description' => t('Test low-level theme template functions.'),
+      'group' => t('Theme'),
+    );
+  }
+
+  /**
+   * Test function template_page_suggestions() for SA-CORE-2009-003.
+   */
+  function testTemplateSuggestions() {
+    // Set the front page as something random otherwise the CLI
+    // test runner fails.
+    variable_set('site_frontpage', 'nobody-home');
+    $args = array('node', '1', 'edit');
+    $suggestions = template_page_suggestions($args);
+    $this->assertEqual($suggestions, array('page-node', 'page-node-1', 'page-node-edit'), t('Found expected node edit page template suggestions'));
+    // Check attack vectors.
+    $args = array('node', '\\1');
+    $suggestions = template_page_suggestions($args);
+    $this->assertEqual($suggestions, array('page-node', 'page-node-1'), t('Removed invalid \\ from template suggestions'));
+    $args = array('node', '1/');
+    $suggestions = template_page_suggestions($args);
+    $this->assertEqual($suggestions, array('page-node', 'page-node-1'), t('Removed invalid / from template suggestions'));
+    $args = array('node', "1\0");
+    $suggestions = template_page_suggestions($args);
+    $this->assertEqual($suggestions, array('page-node', 'page-node-1'), t('Removed invalid \\0 from template suggestions'));
+    // Tests for drupal_discover_template()
+    $suggestions = array('page');
+    $this->assertEqual(drupal_discover_template(array('themes/garland'), $suggestions), 'themes/garland/page.tpl.php', t('Safe template discovered'));
+    $suggestions = array('page');
+    $this->assertEqual(drupal_discover_template(array('themes/garland'), $suggestions, '\\.tpl.php'), 'themes/garland/page.tpl.php', t('Unsafe extension fixed'));
+    $suggestions = array('page\\');
+    $this->assertEqual(drupal_discover_template(array('themes/garland'), $suggestions), 'themes/garland/page.tpl.php', t('Unsafe template suggestion fixed'));
+    $suggestions = array('page/');
+    $this->assertEqual(drupal_discover_template(array('themes/garland'), $suggestions), 'themes/garland/page.tpl.php', t('Unsafe template suggestion fixed'));
+    $suggestions = array("page\0");
+    $this->assertEqual(drupal_discover_template(array('themes/garland'), $suggestions), 'themes/garland/page.tpl.php', t('Unsafe template suggestion fixed'));
+  }
+
+}