Adding SafeString which is going to replace p::clean() and p::purify().

Refactoring of Xss_Security_Test.
t() and t2() return a SafeString instance.

TODO:
 - Update all code to use SafeString where appropriate.
 - Update golden fole of Xss_Security_Test
 - Stop reporting CLEAN vars in Xss_Security_Test

2 files changed, 369 insertions, 67 deletions

diff --git a/modules/gallery/tests/SafeString_Test.php b/modules/gallery/tests/SafeString_Test.php
new file mode 100644
index 00000000..cdae3e99
--- /dev/null
+++ b/modules/gallery/tests/SafeString_Test.php
@@ -0,0 +1,111 @@
+<?php defined("SYSPATH") or die("No direct script access.");
+/**
+ * Gallery - a web based photo album viewer and editor
+ * Copyright (C) 2000-2009 Bharat Mediratta
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+class SafeString_Test extends Unit_Test_Case {
+  public function p_clean_returns_safestring_instance_test() {
+    $safe_string = p::clean("hello <p>world</p>");
+    $this->assert_true($safe_string instanceof SafeString);
+    $this->assert_equal("hello <p>world</p>",
+			$safe_string->unescaped());
+  }
+
+  public function toString_escapes_for_html_test() {
+    $safe_string = new SafeString("hello <p>world</p>");
+    $this->assert_equal("hello &lt;p&gt;world&lt;/p&gt;",
+			$safe_string);
+  }
+
+  public function toString_for_safe_string_test() {
+    $safe_string = new SafeString("hello <p>world</p>");
+    $safe_string->mark_html_safe();
+    $this->assert_equal("hello <p>world</p>",
+			$safe_string);
+  }
+
+  public function for_html_test() {
+    $safe_string = new SafeString("hello <p>world</p>");
+    $this->assert_equal("hello &lt;p&gt;world&lt;/p&gt;",
+			$safe_string->for_html());
+  }
+
+  public function safestring_of_safestring_test() {
+    $safe_string = new SafeString("hello <p>world</p>");
+    $safe_string_2 = new SafeString($safe_string);
+    $this->assert_true($safe_string_2 instanceof SafeString);
+    $raw_string = $safe_string_2->unescaped();
+    $this->assert_false(is_object($raw_string));
+    $this->assert_equal("hello <p>world</p>", $raw_string);
+    $this->assert_equal("hello &lt;p&gt;world&lt;/p&gt;", $safe_string_2);
+  }
+
+  public function for_js_test() {
+    $safe_string = new SafeString('"<em>Foo</em>\'s bar"');
+    $js_string = $safe_string->for_js();
+    $this->assert_equal('\\"<em>Foo<\\/em>\\\'s bar\\"',
+			$js_string);
+  }
+
+  public function string_safestring_equality_test() {
+    $safe_string = new SafeString("hello <p>world</p>");
+    $this->assert_equal("hello <p>world</p>",
+			$safe_string->unescaped());
+    $escaped_string = "hello &lt;p&gt;world&lt;/p&gt;";
+    $this->assert_equal($escaped_string, $safe_string);
+
+    $this->assert_true($escaped_string == $safe_string);
+    $this->assert_false($escaped_string === $safe_string);
+    $this->assert_false("meow" == $safe_string);
+  }
+
+  public function of_test() {
+    $safe_string = SafeString::of("hello <p>world</p>");
+    $this->assert_equal("hello <p>world</p>", $safe_string->unescaped());
+  }
+
+  public function of_safe_html_test() {
+    $safe_string = SafeString::of("hello <p>world</p>")->mark_html_safe();
+    $this->assert_equal("hello <p>world</p>", $safe_string->for_html());
+  }
+
+  public function of_fluid_api_test() {
+    $escaped_string = SafeString::of("Foo's bar")->for_js();
+    $this->assert_equal("Foo\\'s bar", $escaped_string);
+  }
+
+  public function safestring_of_safestring_preserves_safe_status_test() {
+    $safe_string = SafeString::of("hello's <p>world</p>")->mark_html_safe();
+    $safe_string_2 = new SafeString($safe_string);
+    $this->assert_equal("hello's <p>world</p>", $safe_string_2);
+    $this->assert_equal("hello\\'s <p>world<\\/p>", $safe_string_2->for_js());
+  }
+
+  public function safestring_of_safestring_preserves_html_safe_status_test() {
+    $safe_string = SafeString::of("hello's <p>world</p>")
+      ->mark_html_safe();
+    $safe_string_2 = new SafeString($safe_string);
+    $this->assert_equal("hello's <p>world</p>", $safe_string_2);
+    $this->assert_equal("hello\\'s <p>world<\\/p>", $safe_string_2->for_js());
+  }
+
+  public function safestring_of_safestring_safe_status_override_test() {
+    $safe_string = new SafeString("hello <p>world</p>");
+    $safe_string_2 = SafeString::of($safe_string)->mark_html_safe();
+    $this->assert_equal("hello <p>world</p>", $safe_string_2);
+  }
+}
diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php
index 9bde11dc..1d52237c 100644
--- a/modules/gallery/tests/Xss_Security_Test.php
+++ b/modules/gallery/tests/Xss_Security_Test.php
@@ -19,87 +19,278 @@
  */
 class Xss_Security_Test extends Unit_Test_Case {
   public function find_unescaped_variables_in_views_test() {
+    $found = array();
     foreach (glob("*/*/views/*.php") as $view) {
-      $expr = null;
-      $level = 0;
-      $php = 0;
-      $str = null;
-      $in_p_clean = 0;
+      // List of all tokens without whitespace, simplifying parsing.
+      $tokens = array();
       foreach (token_get_all(file_get_contents($view)) as $token) {
-        if (false /* useful for debugging */) {
-          if (is_array($token)) {
-            printf("[$str] [$in_p_clean] %-15s %s\n", token_name($token[0]), $token[1]);
-          } else {
-            printf("[$str] [$in_p_clean] %-15s %s\n", "<char>", $token);
-          }
-        }
-
-        // If we find a "(" after a "p::clean" then start counting levels of parens and assume
-        // that we're inside a p::clean() call until we find the matching close paren.
-        if ($token[0] == "(" && ($str == "p::clean" || $str == "p::purify")) {
-          $in_p_clean = 1;
-        } else if ($token[0] == "(" && $in_p_clean) {
-          $in_p_clean++;
-        } else if ($token[0] == ")" && $in_p_clean) {
-          $in_p_clean--;
-        }
-
-        // Concatenate runs of strings for convenience, which we use above to figure out if we're
-        // inside a p::clean() call or not
-        if ($token[0] == T_STRING || $token[0] == T_DOUBLE_COLON) {
-          $str .= $token[1];
-        } else {
-          $str = null;
-        }
-
-        // Scan for any occurrences of < ? = $variable ? > and store it in $expr
-        if ($token[0] == T_OPEN_TAG_WITH_ECHO) {
-          $php++;
-        } else if ($php && $token[0] == T_CLOSE_TAG) {
-          $php--;
-        } else if ($php && $token[0] == T_VARIABLE) {
-          if (!$expr) {
-            $entry = array($token[2], $in_p_clean);
-          }
-          $expr .= $token[1];
-        } else if ($expr) {
-          if ($token[0] == T_OBJECT_OPERATOR) {
-            $expr .= $token[1];
-          } else if ($token[0] == T_STRING) {
-            $expr .= $token[1];
-          } else if ($token == "(") {
-            $expr .= $token;
-            $level++;
-          } else if ($level > 0 && $token == ")") {
-            $expr .= $token;
-            $level--;
-          } else if ($level > 0) {
-            $expr .= is_array($token) ? $token[1] : $token;
-          } else {
-            $entry[] = $expr;
-            $found[$view][] = $entry;
-            $expr = null;
-            $entry = null;
-          }
-        }
+	if (!is_array($token) || ($token[0] != T_WHITESPACE)) {
+	  $tokens[] = $token;
+	}
+      }
+
+      $frame  = null;
+      $script_block = 0;
+      $in_script_block = false;
+
+      for ($token_number = 0; $token_number < count($tokens); $token_number++) {
+	$token = $tokens[$token_number];
+
+	// Are we in a <script> ... </script> block?
+	if (is_array($token) && $token[0] == T_INLINE_HTML) {
+	  $inline_html = $token[1];
+	  // T_INLINE_HTML blocks can be split. Need to handle the case
+	  // where one token has "<scr" and the next has "ipt"
+	  while (self::_token_matches(array(T_INLINE_HTML), $tokens, $token_number + 1)) {
+	    $token_number++;
+	    $token = $tokens[$token_number];
+	    $inline_html .= $token[1];
+	  }
+
+	  if ($frame) {
+	    $frame->expr_append($inline_html);
+	  }
+
+	  // Note: This approach won't catch <script src="..."> blocks if the src
+	  // URL is generated via < ? = url::site() ? > or some other PHP.
+	  // Assume that all such script blocks with a src URL have an
+	  // empty element body.
+	  // But we'll catch closing tags for such blocks, so don't keep track
+	  // of opening / closing tag count since it would be meaningless.
+
+	  // Handle multiple start / end blocks on the same line?
+	  $opening_script_pos = $closing_script_pos = 0;
+	  if (preg_match_all('{</script>}i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) {
+	    $last_match = array_pop($matches[0]);
+	    if (is_array($last_match)) {
+	      $closing_script_pos = $last_match[1];
+	    } else {
+	      $closing_script_pos = $last_match;
+	    }
+	  }
+	  if (preg_match('{<script\b[^>]*>}i', $inline_html, $matches, PREG_OFFSET_CAPTURE)) {
+	    $last_match = array_pop($matches[0]);
+	    if (is_array($last_match)) {
+	      $opening_script_pos = $last_match[1];
+	    } else {
+	      $opening_script_pos = $last_match;
+	    }
+	  }
+	  if ($opening_script_pos != $closing_script_pos) {
+	    $in_script_block = $opening_script_pos > $closing_script_pos;
+	  }
+	}
+
+	// Look and report each instance of < ? = ... ? >
+	if (!is_array($token)) {
+	  // A single char token, e.g: ; ( )
+	  if ($frame) {
+	    $frame->expr_append($token);
+	  }
+	} else if ($token[0] == T_OPEN_TAG_WITH_ECHO) {
+	  // No need for a stack here - assume < ? = cannot be nested.
+	  $frame = self::_create_frame($token, $in_script_block);
+        } else if ($frame && $token[0] == T_CLOSE_TAG) {
+	  // Store the < ? = ... ? > block that just ended here.
+	  $found[$view][] = $frame;
+	  $frame = null;
+        } else if ($frame && $token[0] == T_VARIABLE) {
+	  $frame->expr_append($token[1]);
+	} else if ($frame && $token[0] == T_STRING) {
+	  $frame->expr_append($token[1]);
+	  // t() and t2() are special in that they're guaranteed to return a SafeString().
+	  if (in_array($token[1], array("t", "t2"))) {
+	    if (self::_token_matches("(", $tokens, $token_number + 1)) {
+	      $frame->is_safestring(true);
+	      $frame->expr_append("(");
+
+	      $token_number++;
+	      $token = $tokens[$token_number];
+	    }
+	  } else if ($token[1] == "SafeString") {
+	    // Looking for SafeString::of(...
+	    if (self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) &&
+		self::_token_matches(array(T_STRING, "of"), $tokens, $token_number + 2)	&&
+		self::_token_matches("(", $tokens, $token_number + 3)) {
+	      $frame->is_safestring(true);
+	      $frame->expr_append("::of(");
+
+	      $token_number += 3;
+	      $token = $tokens[$token_number];
+	    }
+	  } else if ($token[1] == "json_encode") {
+	    if (self::_token_matches("(", $tokens, $token_number + 1)) {
+	      $frame->json_encode_called(true);
+	      $frame->expr_append("(");
+
+	      $token_number++;
+	      $token = $tokens[$token_number];
+	    }
+	  }
+	} else if ($frame && $token[0] == T_OBJECT_OPERATOR) {
+	  $frame->expr_append($token[1]);
+
+	  if (self::_token_matches(array(T_STRING), $tokens, $token_number + 1) &&
+	      in_array($tokens[$token_number + 1][1],
+		       array("for_js", "for_html", "purified_html")) &&
+	      self::_token_matches("(", $tokens, $token_number + 2)) {
+
+	    $method = $tokens[$token_number + 1][1];
+	    $frame->expr_append("$method(");
+
+	    $token_number += 2;
+	    $token = $tokens[$token_number];
+
+	    if ("for_js" == $method) {
+	      $frame->for_js_called(true);
+	    } else if ("for_html" == $method) {
+	      $frame->for_html_called(true);
+	    } else if ("purified_html" == $method) {
+	      $frame->purified_html_called(true);
+	    }
+	  }
+        } else if ($frame) {
+	  $frame->expr_append($token[1]);
+	}
       }
     }
 
-    $canonical = MODPATH . "gallery/tests/xss_data.txt";
+    // Generate the report.
+    /*
+     * States for uses of < ? = X ? >:
+     * JS_XSS:
+     *   In <script> block
+     *     X can be anything without calling ->for_js()
+     * UNKNOWN:
+     *   Outside <script> block:
+     *     X can be anything without a call to ->for_html() or ->purified_html()
+     * CLEAN:
+     *   Outside <script> block:
+     *     X = t() or t2()
+     *     X = * and for_html() or purified_html() is called
+     *   Inside <script> block:
+     *     X = * with ->for_js() or json_encode(...)
+     */
     $new = TMPPATH . "xss_data.txt";
     $fd = fopen($new, "wb");
     ksort($found);
-    foreach ($found as $view => $entries) {
-      foreach ($entries as $entry) {
-        fwrite($fd,
-               sprintf("%-60s %-3s %-5s %s\n",
-                       $view, $entry[0], $entry[1] ? "" : "DIRTY", $entry[2]));
+    foreach ($found as $view => $frames) {
+      foreach ($frames as $frame) {
+	$state = "UNKNOWN";
+	if ($frame->in_script_block()) {
+	  $state = "JS_XSS";
+	  if ($frame->for_js_called() || $frame->json_encode_called()) {
+	    $state = "CLEAN";
+	  }
+	} else {
+	  if ($frame->is_safestring() || $frame->purified_html_called() || $frame->for_html_called()) {
+	    $state = "CLEAN";
+	  }
+	}
+	fprintf($fd, "%-60s %-3s %-8s %s\n",
+		$view, $frame->line(), $state, $frame->expr());
       }
     }
     fclose($fd);
+    exit;
 
+    // Compare with the expected report from our golden file.
+    $canonical = MODPATH . "gallery/tests/xss_data.txt";
     exec("diff $canonical $new", $output, $return_value);
     $this->assert_false(
       $return_value, "XSS golden file mismatch.  Output:\n" . implode("\n", $output) );
   }
+
+  private static function _create_frame($token, $in_script_block) {
+    return new Xss_Security_Test_Frame($token[2], $in_script_block);
+  }
+
+  private static function _token_matches($expected_token, &$tokens, $token_number) {
+    if (!isset($tokens[$token_number])) {
+      return false;
+    }
+
+    $token = $tokens[$token_number];
+
+    if (is_array($expected_token)) {
+      for ($i = 0; $i < count($expected_token); $i++) {
+	if ($expected_token[$i] != $token[$i]) {
+	  return false;
+	}
+      }
+      return true;
+    } else {
+      return $expected_token == $token;
+    }
+  }
+}
+
+class Xss_Security_Test_Frame {
+  private $_expr = "";
+  private $_in_script_block = false;
+  private $_is_safestring = false;
+  private $_for_js_called = false;
+  private $_for_html_called = false;
+  private $_purified_html_called = false;
+  private $_json_encode_called = false;
+  private $_line;
+
+  function __construct($line_number, $in_script_block) {
+    $this->_line = $line_number;
+    $this->in_script_block($in_script_block);
+  }
+
+  function expr() {
+    return $this->_expr;
+  }
+
+  function expr_append($append_value) {
+    return $this->_expr .= $append_value;
+  }
+
+  function in_script_block($new_val=NULL) {
+    if ($new_val !== NULL) {
+      $this->_in_script_block = (bool) $new_val;
+    }
+    return $this->_in_script_block;
+  }
+
+  function is_safestring($new_val=NULL) {
+    if ($new_val !== NULL) {
+      $this->_is_safestring = (bool) $new_val;
+    }
+    return $this->_is_safestring;
+  }
+
+  function json_encode_called($new_val=NULL) {
+    if ($new_val !== NULL) {
+      $this->_json_encode_called = (bool) $new_val;
+    }
+    return $this->_json_encode_called;
+  }
+
+  function for_js_called($new_val=NULL) {
+    if ($new_val !== NULL) {
+      $this->_for_js_called = (bool) $new_val;
+    }
+    return $this->_for_js_called;
+  }
+
+  function for_html_called($new_val=NULL) {
+    if ($new_val !== NULL) {
+      $this->_for_html_called = (bool) $new_val;
+    }
+    return $this->_for_html_called;
+  }
+
+  function purified_html_called($new_val=NULL) {
+    if ($new_val !== NULL) {
+      $this->_purified_html_called = (bool) $new_val;
+    }
+    return $this->_purified_html_called;
+  }
+
+  function line() {
+    return $this->_line;
+  }
 }