diff options
Diffstat (limited to 'modules')
| -rw-r--r-- | modules/comment/views/admin_comments.html.php | 6 | ||||
| -rw-r--r-- | modules/gallery/tests/Xss_Security_Test.php | 47 | ||||
| -rw-r--r-- | modules/gallery/tests/xss_data.txt | 68 | ||||
| -rw-r--r-- | modules/server_add/views/server_add_tree.html.php | 4 |
4 files changed, 77 insertions, 48 deletions
diff --git a/modules/comment/views/admin_comments.html.php b/modules/comment/views/admin_comments.html.php index 801ce2b3..588c3ebc 100644 --- a/modules/comment/views/admin_comments.html.php +++ b/modules/comment/views/admin_comments.html.php @@ -108,12 +108,12 @@ <a href="#"> <img src="<?= $comment->author()->avatar_url(40, $theme->url("images/avatar.jpg", true)) ?>" class="gAvatar" - alt="<?= html::clean($comment->author_name()) ?>" + alt="<?= html::clean_attribute($comment->author_name()) ?>" width="40" height="40" /> </a> - <p><a href="mailto:<?= html::clean($comment->author_email()) ?>" - title="<?= html::clean($comment->author_email()) ?>"> <?= html::clean($comment->author_name()) ?> </a></p> + <p><a href="mailto:<?= html::clean_attribute($comment->author_email()) ?>" + title="<?= html::clean_attribute($comment->author_email()) ?>"> <?= html::clean($comment->author_name()) ?> </a></p> </td> <td> <div class="right"> diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index 6c141c52..ef36f6b7 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -32,6 +32,7 @@ class Xss_Security_Test extends Unit_Test_Case { $frame = null; $script_block = 0; $in_script_block = false; + $inline_html = ""; for ($token_number = 0; $token_number < count($tokens); $token_number++) { $token = $tokens[$token_number]; @@ -81,6 +82,8 @@ class Xss_Security_Test extends Unit_Test_Case { } } + $href_attribute_start = preg_match('{href\s*=\s*[\'"]?\s*$}i', str_replace("\n", "", $inline_html)); + // Look and report each instance of < ? = ... ? > if (!is_array($token)) { // A single char token, e.g: ; ( ) @@ -89,7 +92,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); + $frame = self::_create_frame($token, $in_script_block, $href_attribute_start); + $href_attribute_start = false; } else if ($frame && $token[0] == T_CLOSE_TAG) { // Store the < ? = ... ? > block that just ended here. $found[$view][] = $frame; @@ -177,6 +181,7 @@ class Xss_Security_Test extends Unit_Test_Case { "abs_file", "merge")) && self::_token_matches("(", $tokens, $token_number + 3)) { $frame->is_safe_html(true); + $frame->is_safe_href_attr(true); $method = $tokens[$token_number + 2][1]; $frame->expr_append("::$method("); @@ -237,6 +242,8 @@ class Xss_Security_Test extends Unit_Test_Case { * DIRTY_JS: * In <script> block * X can be anything without calling ->for_js() + * At the start of a href= attribute + * X = anything but a url method * DIRTY: * Outside <script> block: * X can be anything without a call to ->for_html() or ->purified_html() @@ -246,6 +253,8 @@ class Xss_Security_Test extends Unit_Test_Case { * X = * and for_html() or purified_html() is called * Inside <script> block: * X = * with ->for_js() or json_encode(...) + * Start of href attribute: + * X = url method */ $new = TMPPATH . "xss_data.txt"; $fd = fopen($new, "wb"); @@ -253,11 +262,18 @@ class Xss_Security_Test extends Unit_Test_Case { foreach ($found as $view => $frames) { foreach ($frames as $frame) { $state = "DIRTY"; - if ($frame->in_script_block()) { + if ($frame->in_script_block() && $frame->in_href_attribute()) { + $state = "ILLEGAL"; + } else if ($frame->in_script_block()) { $state = "DIRTY_JS"; if ($frame->is_safe_js()) { $state = "CLEAN"; } + } else if ($frame->in_href_attribute()) { + $state = "DIRTY_JS"; + if ($frame->is_safe_href_attr()) { + $state = "CLEAN"; + } } else { if ($frame->is_safe_html()) { $state = "CLEAN"; @@ -283,8 +299,8 @@ 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) { - return new Xss_Security_Test_Frame($token[2], $in_script_block); + 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 _token_matches($expected_token, &$tokens, $token_number) { @@ -312,11 +328,14 @@ class Xss_Security_Test_Frame { private $_in_script_block = false; private $_is_safe_html = false; private $_is_safe_js = false; + private $_in_href_attribute = false; + private $_is_safe_href_attr = false; private $_line; - function __construct($line_number, $in_script_block) { + function __construct($line_number, $in_script_block, $href_attribute_start) { $this->_line = $line_number; - $this->in_script_block($in_script_block); + $this->_in_script_block = $in_script_block; + $this->_in_href_attribute = $href_attribute_start; } function expr() { @@ -327,13 +346,14 @@ class Xss_Security_Test_Frame { return $this->_expr .= $append_value; } - function in_script_block($new_val=NULL) { - if ($new_val !== NULL) { - $this->_in_script_block = (bool) $new_val; - } + function in_script_block() { return $this->_in_script_block; } + function in_href_attribute() { + return $this->_in_href_attribute; + } + function is_safe_html($new_val=NULL) { if ($new_val !== NULL) { $this->_is_safe_html = (bool) $new_val; @@ -341,6 +361,13 @@ class Xss_Security_Test_Frame { return $this->_is_safe_html; } + function is_safe_href_attr($new_val=NULL) { + if ($new_val !== NULL) { + $this->_is_safe_href_attr = (bool) $new_val; + } + return $this->_is_safe_href_attr; + } + function is_safe_js($new_val=NULL) { if ($new_val !== NULL) { $this->_is_safe_js = (bool) $new_val; diff --git a/modules/gallery/tests/xss_data.txt b/modules/gallery/tests/xss_data.txt index 5b43d1e5..5686bf9e 100644 --- a/modules/gallery/tests/xss_data.txt +++ b/modules/gallery/tests/xss_data.txt @@ -6,7 +6,7 @@ modules/comment/views/admin_block_recent_comments.html.php 10 DIRTY galler modules/comment/views/admin_comments.html.php 42 DIRTY $menu modules/comment/views/admin_comments.html.php 106 DIRTY $comment->id modules/comment/views/admin_comments.html.php 106 DIRTY ($i%2==0)?"gOddRow":"gEvenRow" -modules/comment/views/admin_comments.html.php 122 DIRTY $item->url() +modules/comment/views/admin_comments.html.php 122 DIRTY_JS $item->url() 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) @@ -19,9 +19,9 @@ modules/comment/views/admin_comments.html.php 183 DIRTY $comme 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 -modules/comment/views/comment.mrss.php 13 DIRTY $feed->uri -modules/comment/views/comment.mrss.php 16 DIRTY $feed->previous_page_uri -modules/comment/views/comment.mrss.php 19 DIRTY $feed->next_page_uri +modules/comment/views/comment.mrss.php 13 DIRTY_JS $feed->uri +modules/comment/views/comment.mrss.php 16 DIRTY_JS $feed->previous_page_uri +modules/comment/views/comment.mrss.php 19 DIRTY_JS $feed->next_page_uri modules/comment/views/comment.mrss.php 21 DIRTY $pub_date modules/comment/views/comment.mrss.php 22 DIRTY $pub_date modules/comment/views/comment.mrss.php 28 DIRTY $child->item_uri @@ -41,7 +41,7 @@ modules/gallery/views/admin_block_log_entries.html.php 4 DIRTY log::s modules/gallery/views/admin_block_log_entries.html.php 6 DIRTY gallery::date_time($entry->timestamp) modules/gallery/views/admin_block_log_entries.html.php 7 DIRTY $entry->message modules/gallery/views/admin_block_log_entries.html.php 8 DIRTY $entry->html -modules/gallery/views/admin_block_news.html.php 5 DIRTY $entry["link"] +modules/gallery/views/admin_block_news.html.php 5 DIRTY_JS $entry["link"] modules/gallery/views/admin_block_news.html.php 5 DIRTY $entry["title"] modules/gallery/views/admin_block_news.html.php 7 DIRTY text::limit_words(strip_tags($entry["description"]),25); modules/gallery/views/admin_block_photo_stream.html.php 6 DIRTY photo::img_dimensions($photo->width,$photo->height,72) @@ -166,17 +166,17 @@ modules/gallery/views/upgrader.html.php 44 DIRTY $modul modules/gallery/views/upgrader.html.php 45 DIRTY $id modules/gallery/views/upgrader.html.php 49 DIRTY $module->version modules/gallery/views/upgrader.html.php 52 DIRTY $module->code_version -modules/image_block/views/image_block_block.html.php 3 DIRTY $item->url() +modules/image_block/views/image_block_block.html.php 3 DIRTY_JS $item->url() modules/image_block/views/image_block_block.html.php 4 DIRTY $item->thumb_img(array("class"=>"gThumbnail")) modules/info/views/info_block.html.php 22 DIRTY date("M j, Y H:i:s",$item->captured) -modules/info/views/info_block.html.php 29 DIRTY $item->owner->url -modules/notification/views/comment_published.html.php 28 DIRTY $comment->item()->url(array(),true) +modules/info/views/info_block.html.php 29 DIRTY_JS $item->owner->url +modules/notification/views/comment_published.html.php 28 DIRTY_JS $comment->item()->url(array(),true) modules/notification/views/comment_published.html.php 29 DIRTY $comment->item()->url(array(),true) -modules/notification/views/item_added.html.php 16 DIRTY $item->url(array(),true) +modules/notification/views/item_added.html.php 16 DIRTY_JS $item->url(array(),true) modules/notification/views/item_added.html.php 17 DIRTY $item->url(array(),true) -modules/notification/views/item_deleted.html.php 18 DIRTY $item->parent()->url(array(),true) +modules/notification/views/item_deleted.html.php 18 DIRTY_JS $item->parent()->url(array(),true) modules/notification/views/item_deleted.html.php 19 DIRTY $item->parent()->url(array(),true) -modules/notification/views/item_updated.html.php 20 DIRTY $item->url(array(),true) +modules/notification/views/item_updated.html.php 20 DIRTY_JS $item->url(array(),true) modules/notification/views/item_updated.html.php 20 DIRTY $item->url(array(),true) modules/organize/views/organize_dialog.html.php 22 DIRTY $album_tree modules/organize/views/organize_dialog.html.php 29 DIRTY $micro_thumb_grid @@ -198,9 +198,9 @@ 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 modules/rss/views/feed.mrss.php 10 DIRTY $feed->uri -modules/rss/views/feed.mrss.php 13 DIRTY $feed->uri -modules/rss/views/feed.mrss.php 16 DIRTY $feed->previous_page_uri -modules/rss/views/feed.mrss.php 19 DIRTY $feed->next_page_uri +modules/rss/views/feed.mrss.php 13 DIRTY_JS $feed->uri +modules/rss/views/feed.mrss.php 16 DIRTY_JS $feed->previous_page_uri +modules/rss/views/feed.mrss.php 19 DIRTY_JS $feed->next_page_uri modules/rss/views/feed.mrss.php 21 DIRTY $pub_date modules/rss/views/feed.mrss.php 22 DIRTY $pub_date modules/rss/views/feed.mrss.php 28 DIRTY date("D, d M Y H:i:s T",$child->created); @@ -229,14 +229,13 @@ modules/rss/views/feed.mrss.php 71 DIRTY @files modules/rss/views/feed.mrss.php 72 DIRTY $child->height modules/rss/views/feed.mrss.php 73 DIRTY $child->width modules/rss/views/feed.mrss.php 74 DIRTY $child->mime_type -modules/rss/views/rss_block.html.php 6 DIRTY rss::url($url) +modules/rss/views/rss_block.html.php 6 DIRTY_JS rss::url($url) modules/search/views/search.html.php 30 DIRTY $item_class 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 $dir +modules/server_add/views/server_add_tree.html.php 12 DIRTY 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.html.php 25 DIRTY strtr($file,array('"'=>'\\"')) modules/server_add/views/server_add_tree_dialog.html.php 23 DIRTY $tree modules/tag/views/admin_tags.html.php 13 DIRTY $csrf modules/tag/views/admin_tags.html.php 50 DIRTY $tag->id @@ -259,6 +258,7 @@ 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 modules/watermark/views/admin_watermarks.html.php 19 DIRTY $url +themes/admin_default/views/admin.html.php 16 DIRTY_JS $theme->url() themes/admin_default/views/admin.html.php 34 DIRTY $theme->admin_head() themes/admin_default/views/admin.html.php 38 DIRTY $theme->admin_page_top() themes/admin_default/views/admin.html.php 46 DIRTY $theme->admin_header_top() @@ -273,32 +273,34 @@ themes/admin_default/views/block.html.php 2 DIRTY $id themes/admin_default/views/block.html.php 2 DIRTY $css_id themes/admin_default/views/block.html.php 10 DIRTY $title themes/admin_default/views/block.html.php 13 DIRTY $content -themes/admin_default/views/pager.html.php 13 DIRTY str_replace('{page}',1,$url) -themes/admin_default/views/pager.html.php 20 DIRTY str_replace('{page}',$previous_page,$url) +themes/admin_default/views/pager.html.php 13 DIRTY_JS str_replace('{page}',1,$url) +themes/admin_default/views/pager.html.php 20 DIRTY_JS str_replace('{page}',$previous_page,$url) themes/admin_default/views/pager.html.php 27 DIRTY $from_to_msg -themes/admin_default/views/pager.html.php 30 DIRTY str_replace('{page}',$next_page,$url) -themes/admin_default/views/pager.html.php 37 DIRTY str_replace('{page}',$last_page,$url) +themes/admin_default/views/pager.html.php 30 DIRTY_JS str_replace('{page}',$next_page,$url) +themes/admin_default/views/pager.html.php 37 DIRTY_JS str_replace('{page}',$last_page,$url) themes/default/views/album.html.php 16 DIRTY $child->id themes/default/views/album.html.php 16 DIRTY $item_class -themes/default/views/album.html.php 18 DIRTY $child->url() +themes/default/views/album.html.php 18 DIRTY_JS $child->url() themes/default/views/album.html.php 19 DIRTY $child->thumb_img(array("class"=>"gThumbnail")) -themes/default/views/album.html.php 23 DIRTY $child->url() +themes/default/views/album.html.php 23 DIRTY_JS $child->url() themes/default/views/block.html.php 2 DIRTY $anchor themes/default/views/block.html.php 3 DIRTY $css_id themes/default/views/block.html.php 4 DIRTY $title themes/default/views/block.html.php 6 DIRTY $content themes/default/views/dynamic.html.php 11 DIRTY $child->is_album()?"gAlbum":"" -themes/default/views/dynamic.html.php 13 DIRTY $child->url() +themes/default/views/dynamic.html.php 13 DIRTY_JS $child->url() themes/default/views/dynamic.html.php 14 DIRTY $child->id 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 $previous_item->url() -themes/default/views/movie.html.php 18 DIRTY $next_item->url() +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}")) themes/default/views/page.html.php 9 DIRTY $page_title +themes/default/views/page.html.php 26 DIRTY_JS $theme->url() +themes/default/views/page.html.php 32 DIRTY_JS $theme->url() 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 @@ -306,14 +308,14 @@ themes/default/views/page.html.php 79 DIRTY newVie 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/pager.html.php 13 DIRTY str_replace('{page}',1,$url) -themes/default/views/pager.html.php 20 DIRTY str_replace('{page}',$previous_page,$url) +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 -themes/default/views/pager.html.php 30 DIRTY str_replace('{page}',$next_page,$url) -themes/default/views/pager.html.php 37 DIRTY str_replace('{page}',$last_page,$url) +themes/default/views/pager.html.php 30 DIRTY_JS str_replace('{page}',$next_page,$url) +themes/default/views/pager.html.php 37 DIRTY_JS str_replace('{page}',$last_page,$url) themes/default/views/photo.html.php 8 DIRTY_JS $theme->item()->width themes/default/views/photo.html.php 8 DIRTY_JS $theme->item()->height -themes/default/views/photo.html.php 21 DIRTY $previous_item->url() -themes/default/views/photo.html.php 31 DIRTY $next_item->url() -themes/default/views/photo.html.php 43 DIRTY $item->file_url() +themes/default/views/photo.html.php 21 DIRTY_JS $previous_item->url() +themes/default/views/photo.html.php 31 DIRTY_JS $next_item->url() +themes/default/views/photo.html.php 43 DIRTY_JS $item->file_url() themes/default/views/photo.html.php 45 DIRTY $item->resize_img(array("id"=>"gPhotoId-{$item->id}","class"=>"gResize")) diff --git a/modules/server_add/views/server_add_tree.html.php b/modules/server_add/views/server_add_tree.html.php index dbae42c5..918fbdc7 100644 --- a/modules/server_add/views/server_add_tree.html.php +++ b/modules/server_add/views/server_add_tree.html.php @@ -9,7 +9,7 @@ <? foreach ($parents as $dir): ?> <li class="ui-icon-left"> <span class="ui-icon ui-icon-folder-open"></span> - <span ondblclick="open_dir('<?= $dir ?>')"> + <span ondblclick='open_dir(<?= html::js_string($dir) ?>)'> <?= html::clean(basename($dir)) ?> </span> <ul> @@ -22,7 +22,7 @@ <? if (is_dir($file)): ?> ondblclick="open_dir($(this).attr('file'))" <? endif ?> - file="<?= strtr($file, array('"' => '\\"')) ?>" + file="<?= html::clean_attribute($file) ?>" > <?= html::clean(basename($file)) ?> </span> |