Adding XSS test for href="javascript: and onclick="..."

2 files changed, 104 insertions, 67 deletions

diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php
index 0ba5a587..1d1acce8 100644
--- a/modules/gallery/tests/Xss_Security_Test.php
+++ b/modules/gallery/tests/Xss_Security_Test.php
@@ -33,6 +33,8 @@ class Xss_Security_Test extends Unit_Test_Case {
       $script_block = 0;
       $in_script_block = false;
       $inline_html = "";
+      $in_attribute_js_context = false;
+      $href_attribute_start = false;
 
       for ($token_number = 0; $token_number < count($tokens); $token_number++) {
         $token = $tokens[$token_number];
@@ -48,6 +50,8 @@ class Xss_Security_Test extends Unit_Test_Case {
             $inline_html .= $token[1];
           }
 
+          $inline_html = str_replace("\n", " ", $inline_html);
+
           if ($frame) {
             $frame->expr_append($inline_html);
           }
@@ -82,7 +86,23 @@ class Xss_Security_Test extends Unit_Test_Case {
           }
         }
 
-        $href_attribute_start = preg_match('{href\s*=\s*[\'"]?\s*$}i', str_replace("\n", "", $inline_html));
+        $href_attribute_start = preg_match('{\bhref\s*=\s*[\'"]?\s*$}i', $inline_html);
+
+        $pos = false;
+        if ($in_attribute_js_context && ($pos = strpos($inline_html, $delimiter)) !== false) {
+          $in_attribute_js_context = false;
+        }
+        if (!$in_attribute_js_context) {
+          $pos = ($pos === false) ? 0 : $pos;
+          if (preg_match('{\bhref\s*=\s*(")javascript:[^"]*$}i', $inline_html, $matches, 0, $pos) ||
+              preg_match("{\bhref\s*=\s*(')javascript:[^']*$}i", $inline_html, $matches, 0, $pos) ||
+              preg_match("{\bon[a-z]+\s*=\s*(')[^']*$}i", $inline_html, $matches, 0, $pos) ||
+              preg_match('{\bon[a-z]+\s*=\s*(")[^"]*$}i', $inline_html, $matches, 0, $pos)) {
+            $in_attribute_js_context = true;
+            $delimiter = $matches[1];
+            $inline_html = "";
+          }
+        }
 
         // Look and report each instance of < ? = ... ? >
         if (!is_array($token)) {
@@ -92,7 +112,8 @@ class Xss_Security_Test extends Unit_Test_Case {
           }
         } 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, $href_attribute_start);
+          $frame = self::_create_frame($token, $in_script_block,
+                                       $href_attribute_start, $in_attribute_js_context);
           $href_attribute_start = false;
         } else if ($frame && $token[0] == T_CLOSE_TAG) {
           // Store the < ? = ... ? > block that just ended here.
@@ -244,6 +265,8 @@ class Xss_Security_Test extends Unit_Test_Case {
      *     X can be anything without calling ->for_js()
      *   At the start of a href= attribute
      *     X = anything but a url method
+     *   In href="javascript: or onclick="...":
+     *     X = anything (manual review required)
      * DIRTY:
      *   Outside <script> block:
      *     X can be anything without a call to ->for_html() or ->purified_html()
@@ -263,12 +286,16 @@ class Xss_Security_Test extends Unit_Test_Case {
       foreach ($frames as $frame) {
         $state = "DIRTY";
         if ($frame->in_script_block() && $frame->in_href_attribute()) {
+          // This parser assumes this state does not occur.
           $state = "ILLEGAL";
         } else if ($frame->in_script_block()) {
           $state = "DIRTY_JS";
           if ($frame->is_safe_js()) {
             $state = "CLEAN";
           }
+        } else if ($frame->in_attribute_js_context()) {
+          // Manual review required
+          $state = "DIRTY_JS";
         } else if ($frame->in_href_attribute()) {
           $state = "DIRTY_JS";
           if ($frame->is_safe_href_attr()) {
@@ -299,8 +326,10 @@ class Xss_Security_Test extends Unit_Test_Case {
                         $return_value, "XSS golden file mismatch.  Output:\n" . implode("\n", $output) );
   }
 
-  private static function _create_frame($token, $in_script_block, $href_attribute_start) {
-    return new Xss_Security_Test_Frame($token[2], $in_script_block, $href_attribute_start);
+  private static function _create_frame($token, $in_script_block,
+                                        $href_attribute_start, $in_attribute_js_context) {
+    return new Xss_Security_Test_Frame($token[2], $in_script_block,
+                                       $href_attribute_start, $in_attribute_js_context);
   }
 
   private static function _token_matches($expected_token, &$tokens, $token_number) {
@@ -330,12 +359,15 @@ class Xss_Security_Test_Frame {
   private $_is_safe_js = false;
   private $_in_href_attribute = false;
   private $_is_safe_href_attr = false;
+  private $_in_attribute_js_context = false;
   private $_line;
 
-  function __construct($line_number, $in_script_block, $href_attribute_start) {
+  function __construct($line_number, $in_script_block,
+                       $href_attribute_start, $in_attribute_js_context) {
     $this->_line = $line_number;
     $this->_in_script_block = $in_script_block;
     $this->_in_href_attribute = $href_attribute_start;
+    $this->_in_attribute_js_context = $in_attribute_js_context;
   }
 
   function expr() {
@@ -354,6 +386,10 @@ class Xss_Security_Test_Frame {
     return $this->_in_href_attribute;
   }
 
+  function in_attribute_js_context() {
+    return $this->_in_attribute_js_context;
+  }
+
   function is_safe_html($new_val=NULL) {
     if ($new_val !== NULL) {
       $this->_is_safe_html = (bool) $new_val;
diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt
index 5686bf9e..b22114a4 100644
--- a/modules/gallery/tests/xss_data.txt
+++ b/modules/gallery/tests/xss_data.txt
@@ -10,12 +10,12 @@ modules/comment/views/admin_comments.html.php                122 DIRTY_JS $item-
 modules/comment/views/admin_comments.html.php                124 DIRTY    $item->thumb_url()
 modules/comment/views/admin_comments.html.php                126 DIRTY    photo::img_dimensions($item->thumb_width,$item->thumb_height,75)
 modules/comment/views/admin_comments.html.php                134 DIRTY    gallery::date($comment->created)
-modules/comment/views/admin_comments.html.php                141 DIRTY    $comment->id
-modules/comment/views/admin_comments.html.php                150 DIRTY    $comment->id
-modules/comment/views/admin_comments.html.php                159 DIRTY    $comment->id
-modules/comment/views/admin_comments.html.php                168 DIRTY    $comment->id
-modules/comment/views/admin_comments.html.php                175 DIRTY    $comment->id
-modules/comment/views/admin_comments.html.php                183 DIRTY    $comment->id
+modules/comment/views/admin_comments.html.php                141 DIRTY_JS $comment->id
+modules/comment/views/admin_comments.html.php                150 DIRTY_JS $comment->id
+modules/comment/views/admin_comments.html.php                159 DIRTY_JS $comment->id
+modules/comment/views/admin_comments.html.php                168 DIRTY_JS $comment->id
+modules/comment/views/admin_comments.html.php                175 DIRTY_JS $comment->id
+modules/comment/views/admin_comments.html.php                183 DIRTY_JS $comment->id
 modules/comment/views/admin_comments.html.php                196 DIRTY    $pager
 modules/comment/views/comment.html.php                       2   DIRTY    $comment->id;
 modules/comment/views/comment.mrss.php                       10  DIRTY    $feed->uri
@@ -69,20 +69,23 @@ modules/gallery/views/admin_languages.html.php               31  DIRTY    form::
 modules/gallery/views/admin_languages.html.php               102 DIRTY    $share_translations_form
 modules/gallery/views/admin_maintenance.html.php             24  DIRTY    log::severity_class($task->severity)
 modules/gallery/views/admin_maintenance.html.php             24  DIRTY    ($i%2==0)?"gOddRow":"gEvenRow"
+modules/gallery/views/admin_maintenance.html.php             25  DIRTY    log::severity_class($task->severity)
 modules/gallery/views/admin_maintenance.html.php             26  DIRTY    $task->name
 modules/gallery/views/admin_maintenance.html.php             29  DIRTY    $task->description
+modules/gallery/views/admin_maintenance.html.php             72  DIRTY    $task->state=="stalled"?"gWarning":""
+modules/gallery/views/admin_maintenance.html.php             72  DIRTY    ($i%2==0)?"gOddRow":"gEvenRow"
 modules/gallery/views/admin_maintenance.html.php             73  DIRTY    $task->state=="stalled"?"gWarning":""
-modules/gallery/views/admin_maintenance.html.php             73  DIRTY    ($i%2==0)?"gOddRow":"gEvenRow"
-modules/gallery/views/admin_maintenance.html.php             75  DIRTY    gallery::date_time($task->updated)
-modules/gallery/views/admin_maintenance.html.php             78  DIRTY    $task->name
-modules/gallery/views/admin_maintenance.html.php             93  DIRTY    $task->status
-modules/gallery/views/admin_maintenance.html.php             147 DIRTY    $task->state=="success"?"gSuccess":"gError"
-modules/gallery/views/admin_maintenance.html.php             147 DIRTY    ($i%2==0)?"gOddRow":"gEvenRow"
-modules/gallery/views/admin_maintenance.html.php             149 DIRTY    gallery::date_time($task->updated)
-modules/gallery/views/admin_maintenance.html.php             152 DIRTY    $task->name
-modules/gallery/views/admin_maintenance.html.php             164 DIRTY    $task->status
+modules/gallery/views/admin_maintenance.html.php             74  DIRTY    gallery::date_time($task->updated)
+modules/gallery/views/admin_maintenance.html.php             77  DIRTY    $task->name
+modules/gallery/views/admin_maintenance.html.php             92  DIRTY    $task->status
+modules/gallery/views/admin_maintenance.html.php             145 DIRTY    $task->state=="success"?"gSuccess":"gError"
+modules/gallery/views/admin_maintenance.html.php             145 DIRTY    ($i%2==0)?"gOddRow":"gEvenRow"
+modules/gallery/views/admin_maintenance.html.php             146 DIRTY    $task->state=="success"?"gSuccess":"gError"
+modules/gallery/views/admin_maintenance.html.php             147 DIRTY    gallery::date_time($task->updated)
+modules/gallery/views/admin_maintenance.html.php             150 DIRTY    $task->name
+modules/gallery/views/admin_maintenance.html.php             162 DIRTY    $task->status
 modules/gallery/views/admin_maintenance_show_log.html.php    13  DIRTY    $task->name
-modules/gallery/views/admin_maintenance_task.html.php        54  DIRTY    $task->name
+modules/gallery/views/admin_maintenance_task.html.php        55  DIRTY    $task->name
 modules/gallery/views/admin_modules.html.php                 9   DIRTY    access::csrf_form_field()
 modules/gallery/views/admin_modules.html.php                 19  DIRTY    ($i%2==0)?"gOddRow":"gEvenRow"
 modules/gallery/views/admin_modules.html.php                 22  DIRTY    form::checkbox($data,'1',module::is_active($module_name))
@@ -123,45 +126,45 @@ modules/gallery/views/maintenance.html.php                   46  DIRTY    user::
 modules/gallery/views/move_browse.html.php                   39  DIRTY    $tree
 modules/gallery/views/move_browse.html.php                   43  DIRTY    access::csrf_form_field()
 modules/gallery/views/move_tree.html.php                     2   DIRTY    $parent->thumb_img(array(),25);
-modules/gallery/views/move_tree.html.php                     4   DIRTY    $parent->id
-modules/gallery/views/move_tree.html.php                     6   DIRTY    $parent->id
+modules/gallery/views/move_tree.html.php                     4   DIRTY_JS $parent->id
+modules/gallery/views/move_tree.html.php                     6   DIRTY_JS $parent->id
 modules/gallery/views/move_tree.html.php                     8   DIRTY    $parent->id
 modules/gallery/views/move_tree.html.php                     10  DIRTY    $child->id
 modules/gallery/views/move_tree.html.php                     11  DIRTY    $child->thumb_img(array(),25);
-modules/gallery/views/move_tree.html.php                     13  DIRTY    $child->id
-modules/gallery/views/move_tree.html.php                     15  DIRTY    $child->id
+modules/gallery/views/move_tree.html.php                     13  DIRTY_JS $child->id
+modules/gallery/views/move_tree.html.php                     15  DIRTY_JS $child->id
 modules/gallery/views/movieplayer.html.php                   2   DIRTY    html::anchor($item->file_url(true),"",$attrs)
 modules/gallery/views/movieplayer.html.php                   5   DIRTY    $attrs["id"]
 modules/gallery/views/permissions_browse.html.php            41  DIRTY    $parent->id
-modules/gallery/views/permissions_browse.html.php            42  DIRTY    $parent->id
+modules/gallery/views/permissions_browse.html.php            42  DIRTY_JS $parent->id
 modules/gallery/views/permissions_browse.html.php            47  DIRTY    $item->id
-modules/gallery/views/permissions_browse.html.php            48  DIRTY    $item->id
+modules/gallery/views/permissions_browse.html.php            48  DIRTY_JS $item->id
 modules/gallery/views/permissions_browse.html.php            55  DIRTY    $form
-modules/gallery/views/permissions_form.html.php              24  DIRTY    $lock->id
-modules/gallery/views/permissions_form.html.php              32  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              32  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              32  DIRTY    $item->id
-modules/gallery/views/permissions_form.html.php              36  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              36  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              36  DIRTY    $item->id
-modules/gallery/views/permissions_form.html.php              43  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              43  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              43  DIRTY    $item->id
-modules/gallery/views/permissions_form.html.php              47  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              47  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              47  DIRTY    $item->id
-modules/gallery/views/permissions_form.html.php              56  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              56  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              56  DIRTY    $item->id
-modules/gallery/views/permissions_form.html.php              63  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              63  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              63  DIRTY    $item->id
-modules/gallery/views/permissions_form.html.php              74  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              74  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              74  DIRTY    $item->id
-modules/gallery/views/permissions_form.html.php              79  DIRTY    $group->id
-modules/gallery/views/permissions_form.html.php              79  DIRTY    $permission->id
-modules/gallery/views/permissions_form.html.php              79  DIRTY    $item->id
+modules/gallery/views/permissions_form.html.php              24  DIRTY_JS $lock->id
+modules/gallery/views/permissions_form.html.php              32  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              32  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              32  DIRTY_JS $item->id
+modules/gallery/views/permissions_form.html.php              36  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              36  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              36  DIRTY_JS $item->id
+modules/gallery/views/permissions_form.html.php              43  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              43  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              43  DIRTY_JS $item->id
+modules/gallery/views/permissions_form.html.php              47  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              47  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              47  DIRTY_JS $item->id
+modules/gallery/views/permissions_form.html.php              56  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              56  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              56  DIRTY_JS $item->id
+modules/gallery/views/permissions_form.html.php              63  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              63  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              63  DIRTY_JS $item->id
+modules/gallery/views/permissions_form.html.php              74  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              74  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              74  DIRTY_JS $item->id
+modules/gallery/views/permissions_form.html.php              79  DIRTY_JS $group->id
+modules/gallery/views/permissions_form.html.php              79  DIRTY_JS $permission->id
+modules/gallery/views/permissions_form.html.php              79  DIRTY_JS $item->id
 modules/gallery/views/upgrader.html.php                      44  DIRTY    $module->version==$module->code_version?"current":"upgradeable"
 modules/gallery/views/upgrader.html.php                      45  DIRTY    $id
 modules/gallery/views/upgrader.html.php                      49  DIRTY    $module->version
@@ -188,12 +191,12 @@ modules/organize/views/organize_thumb_grid.html.php          5   DIRTY    $child
 modules/organize/views/organize_thumb_grid.html.php          6   DIRTY    $child->thumb_img(array("class"=>"gThumbnail","ref"=>$child->id),90,true)
 modules/organize/views/organize_tree.html.php                2   DIRTY    access::can("edit",$album)?"":"gViewOnly"
 modules/organize/views/organize_tree.html.php                3   DIRTY    $album->id
-modules/organize/views/organize_tree.html.php                7   DIRTY    $selected&&$album->id==$selected->id?"selected":""
-modules/organize/views/organize_tree.html.php                9   DIRTY    $album->id
-modules/organize/views/organize_tree.html.php                15  DIRTY    View::factory("organize_tree.html",array("selected"=>$selected,"album"=>$child));
-modules/organize/views/organize_tree.html.php                17  DIRTY    access::can("edit",$child)?"":"gViewOnly"
-modules/organize/views/organize_tree.html.php                18  DIRTY    $child->id
-modules/organize/views/organize_tree.html.php                21  DIRTY    $child->id
+modules/organize/views/organize_tree.html.php                6   DIRTY    $selected&&$album->id==$selected->id?"selected":""
+modules/organize/views/organize_tree.html.php                7   DIRTY    $album->id
+modules/organize/views/organize_tree.html.php                13  DIRTY    View::factory("organize_tree.html",array("selected"=>$selected,"album"=>$child));
+modules/organize/views/organize_tree.html.php                15  DIRTY    access::can("edit",$child)?"":"gViewOnly"
+modules/organize/views/organize_tree.html.php                16  DIRTY    $child->id
+modules/organize/views/organize_tree.html.php                19  DIRTY    $child->id
 modules/recaptcha/views/admin_recaptcha.html.php             10  DIRTY    $form
 modules/recaptcha/views/admin_recaptcha.html.php             23  DIRTY    $public_key
 modules/recaptcha/views/form_recaptcha.html.php              7   DIRTY    $public_key
@@ -234,7 +237,7 @@ modules/search/views/search.html.php                         30  DIRTY    $item_
 modules/search/views/search.html.php                         32  DIRTY    $item->thumb_img()
 modules/server_add/views/admin_server_add.html.php           15  DIRTY    $id
 modules/server_add/views/admin_server_add.html.php           24  DIRTY    $form
-modules/server_add/views/server_add_tree.html.php            12  DIRTY    html::js_string($dir)
+modules/server_add/views/server_add_tree.html.php            12  DIRTY_JS html::js_string($dir)
 modules/server_add/views/server_add_tree.html.php            20  DIRTY    is_dir($file)?"ui-icon-folder-collapsed":"ui-icon-document"
 modules/server_add/views/server_add_tree_dialog.html.php     23  DIRTY    $tree
 modules/tag/views/admin_tags.html.php                        13  DIRTY    $csrf
@@ -252,8 +255,8 @@ modules/user/views/admin_users.html.php                      83  DIRTY    ($user
 modules/user/views/admin_users.html.php                      121 DIRTY    $group->id
 modules/user/views/admin_users.html.php                      121 DIRTY    ($group->special?"gDefaultGroup":"")
 modules/user/views/admin_users.html.php                      123 DIRTY    $v
-modules/user/views/admin_users_group.html.php                22  DIRTY    $user->id
-modules/user/views/admin_users_group.html.php                22  DIRTY    $group->id
+modules/user/views/admin_users_group.html.php                22  DIRTY_JS $user->id
+modules/user/views/admin_users_group.html.php                22  DIRTY_JS $group->id
 modules/user/views/login_ajax.html.php                       37  DIRTY    $form
 modules/watermark/views/admin_watermarks.html.php            19  DIRTY    $width
 modules/watermark/views/admin_watermarks.html.php            19  DIRTY    $height
@@ -293,8 +296,6 @@ themes/default/views/dynamic.html.php                        14  DIRTY    $child
 themes/default/views/dynamic.html.php                        15  DIRTY    $child->thumb_url()
 themes/default/views/dynamic.html.php                        16  DIRTY    $child->thumb_width
 themes/default/views/dynamic.html.php                        17  DIRTY    $child->thumb_height
-themes/default/views/footer.html.php                         4   DIRTY    $footer_text
-themes/default/views/header.html.php                         5   DIRTY    $header_text
 themes/default/views/movie.html.php                          8   DIRTY_JS $previous_item->url()
 themes/default/views/movie.html.php                          18  DIRTY_JS $next_item->url()
 themes/default/views/movie.html.php                          28  DIRTY    $item->movie_img(array("class"=>"gMovie","id"=>"gMovieId-{$item->id}"))
@@ -304,10 +305,10 @@ themes/default/views/page.html.php                           32  DIRTY_JS $theme
 themes/default/views/page.html.php                           41  DIRTY    $new_width
 themes/default/views/page.html.php                           42  DIRTY    $new_height
 themes/default/views/page.html.php                           43  DIRTY    $thumb_proportion
-themes/default/views/page.html.php                           79  DIRTY    newView("header.html")
-themes/default/views/page.html.php                           86  DIRTY    $content
-themes/default/views/page.html.php                           92  DIRTY    newView("sidebar.html")
-themes/default/views/page.html.php                           97  DIRTY    newView("footer.html")
+themes/default/views/page.html.php                           82  DIRTY    $header_text
+themes/default/views/page.html.php                           112 DIRTY    $content
+themes/default/views/page.html.php                           118 DIRTY    newView("sidebar.html")
+themes/default/views/page.html.php                           125 DIRTY    $footer_text
 themes/default/views/pager.html.php                          13  DIRTY_JS str_replace('{page}',1,$url)
 themes/default/views/pager.html.php                          20  DIRTY_JS str_replace('{page}',$previous_page,$url)
 themes/default/views/pager.html.php                          27  DIRTY    $from_to_msg