From c01ac42c4604b3b129e8089e0dc683ebd418b380 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sat, 29 Aug 2009 12:48:40 -0700 Subject: Refactor all calls of p::clean() to SafeString::of() and p::purify() to SafeString::purify(). Removing any p::clean() calls for arguments to t() and t2() since their args are wrapped in a SafeString anyway. --- system/helpers/request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'system/helpers') diff --git a/system/helpers/request.php b/system/helpers/request.php index 4203d0e5..15b8edfa 100644 --- a/system/helpers/request.php +++ b/system/helpers/request.php @@ -30,7 +30,7 @@ class request_Core { // Set referrer $ref = $_SERVER['HTTP_REFERER']; - if (strpos($ref, url::base(FALSE)) === 0) + if (strpos($ref, (string) url::base(FALSE)) === 0) { // Remove the base URL from the referrer $ref = substr($ref, strlen(url::base(FALSE))); -- cgit v1.2.3 From b4b638be44375c93f5222c7b48ed547845d6d7e5 Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Sat, 29 Aug 2009 16:28:30 -0700 Subject: Undo url helper changes - url methods no longer return a SafeString. Adding SafeString::of_safe_html() calls where urls are passed as parameters to t() and t2(). --- modules/akismet/helpers/akismet.php | 2 +- modules/digibug/views/admin_digibug.html.php | 2 +- modules/exif/helpers/exif.php | 2 +- modules/g2_import/views/admin_g2_import.html.php | 10 ++++----- modules/gallery/helpers/MY_url.php | 24 +--------------------- modules/gallery/helpers/graphics.php | 2 +- modules/gallery/tests/Xss_Security_Test.php | 15 +++++++++++--- modules/gallery/views/admin_block_welcome.html.php | 10 ++++----- modules/gallery/views/upgrader.html.php | 2 +- modules/recaptcha/helpers/recaptcha.php | 2 +- modules/search/helpers/search.php | 2 +- modules/server_add/helpers/server_add.php | 2 +- modules/user/views/reset_password.html.php | 4 +++- system/helpers/request.php | 2 +- 14 files changed, 35 insertions(+), 46 deletions(-) (limited to 'system/helpers') diff --git a/modules/akismet/helpers/akismet.php b/modules/akismet/helpers/akismet.php index db45a6ab..abca78d2 100644 --- a/modules/akismet/helpers/akismet.php +++ b/modules/akismet/helpers/akismet.php @@ -94,7 +94,7 @@ class akismet_Core { if (empty($api_key)) { site_status::warning( t("Akismet is not quite ready! Please provide an API Key", - array("url" => url::site("admin/akismet"))), + array("url" => SafeString::of_safe_html(url::site("admin/akismet")))), "akismet_config"); } else { site_status::clear("akismet_config"); diff --git a/modules/digibug/views/admin_digibug.html.php b/modules/digibug/views/admin_digibug.html.php index 7e4436ff..5f27a3fd 100644 --- a/modules/digibug/views/admin_digibug.html.php +++ b/modules/digibug/views/admin_digibug.html.php @@ -16,7 +16,7 @@

register with Digibug and enter your Digibug id in the Advanced Settings page you can make money off of your photos!", array("signup_url" => "http://www.digibug.com/signup.php", - "advanced_settings_url" => url::site("admin/advanced_settings"))) ?> + "advanced_settings_url" => SafeString::of_safe_html(url::site("admin/advanced_settings")))) ?>

diff --git a/modules/exif/helpers/exif.php b/modules/exif/helpers/exif.php index 20ecd0cb..d4e60338 100644 --- a/modules/exif/helpers/exif.php +++ b/modules/exif/helpers/exif.php @@ -164,7 +164,7 @@ class exif_Core { if ($remaining) { site_status::warning( t('Your Exif index needs to be updated. Fix this now', - array("url" => url::site("admin/maintenance/start/exif_task::update_index?csrf=__CSRF__"))), + array("url" => SafeString::of_safe_html(url::site("admin/maintenance/start/exif_task::update_index?csrf=__CSRF__")))), "exif_index_out_of_date"); } } diff --git a/modules/g2_import/views/admin_g2_import.html.php b/modules/g2_import/views/admin_g2_import.html.php index da2bb5d1..f53510f6 100644 --- a/modules/g2_import/views/admin_g2_import.html.php +++ b/modules/g2_import/views/admin_g2_import.html.php @@ -37,9 +37,9 @@
  • Using the same value will speed up your import.", - array("g2_pixels" => $g2_sizes["thumb"]["size"], - "g3_pixels" => $thumb_size, - "url" => url::site("admin/theme_options"))) ?> + array("g2_pixels" => $g2_sizes["thumb"]["size"], + "g3_pixels" => $thumb_size, + "url" => SafeString::of_safe_html(url::site("admin/theme_options")))) ?>
    • @@ -47,8 +47,8 @@
  • Using the same value will speed up your import.", array("g2_pixels" => $g2_sizes["resize"]["size"], - "g3_pixels" => $resize_size, - "url" => url::site("admin/theme_options"))) ?> + "g3_pixels" => $resize_size, + "url" => SafeString::of_safe_html(url::site("admin/theme_options")))) ?>
    • diff --git a/modules/gallery/helpers/MY_url.php b/modules/gallery/helpers/MY_url.php index 6092a9d8..c4967c52 100644 --- a/modules/gallery/helpers/MY_url.php +++ b/modules/gallery/helpers/MY_url.php @@ -30,8 +30,7 @@ class url extends url_Core { if ($parts[0] == "albums" || $parts[0] == "photos") { $uri = model_cache::get("item", $parts[1])->relative_path(); } - $url = parent::site($uri . $query, $protocol); - return SafeString::of_safe_html($url); + return parent::site($uri . $query, $protocol); } static function parse_url() { @@ -100,25 +99,4 @@ class url extends url_Core { static function abs_current($qs=false) { return self::abs_site(url::current($qs)); } - - public static function base($index=false, $protocol=false) { - $url = parent::base($index, $protocol); - return SafeString::of_safe_html($url); - } - - public static function current($qs=false) { - $url = parent::current($qs); - return SafeString::of_safe_html($url); - } - - public static function file($file, $index=false) { - $url = parent::file($file, $index); - return SafeString::of_safe_html($url); - } - - public static function merge(array $arguments) { - $url = parent::merge($arguments); - return SafeString::of_safe_html($url); - } - } diff --git a/modules/gallery/helpers/graphics.php b/modules/gallery/helpers/graphics.php index 7dc46eeb..fbb85bec 100644 --- a/modules/gallery/helpers/graphics.php +++ b/modules/gallery/helpers/graphics.php @@ -442,7 +442,7 @@ class graphics_Core { if (!module::get_var("gallery", "graphics_toolkit")) { site_status::warning( t("Graphics toolkit missing! Please choose a toolkit", - array("url" => url::site("admin/graphics"))), + array("url" => SafeString::of_safe_html(url::site("admin/graphics")))), "missing_graphics_toolkit"); } } diff --git a/modules/gallery/tests/Xss_Security_Test.php b/modules/gallery/tests/Xss_Security_Test.php index 690dc760..a2d3d59b 100644 --- a/modules/gallery/tests/Xss_Security_Test.php +++ b/modules/gallery/tests/Xss_Security_Test.php @@ -130,14 +130,14 @@ class Xss_Security_Test extends Unit_Test_Case { $token = $tokens[$token_number]; } } else if ($token[1] == "url") { - // url methods return a SafeString + // url methods return safe HTML if (self::_token_matches(array(T_DOUBLE_COLON, "::"), $tokens, $token_number + 1) && self::_token_matches(array(T_STRING), $tokens, $token_number + 2) && in_array($tokens[$token_number + 2][1], array("site", "current", "base", "file", "abs_site", "abs_current", "abs_file", "merge")) && self::_token_matches("(", $tokens, $token_number + 3)) { - $frame->is_safestring(true); + $frame->is_safe_html(true); $method = $tokens[$token_number + 2][1]; $frame->expr_append("::$method("); @@ -203,7 +203,8 @@ class Xss_Security_Test extends Unit_Test_Case { $state = "CLEAN"; } } else { - if ($frame->is_safestring() || $frame->purified_html_called() || $frame->for_html_called()) { + if ($frame->is_safe_html() || $frame->is_safestring() || + $frame->purified_html_called() || $frame->for_html_called()) { $state = "CLEAN"; } } @@ -259,6 +260,7 @@ class Xss_Security_Test_Frame { private $_for_html_called = false; private $_purified_html_called = false; private $_json_encode_called = false; + private $_is_safe_html = false; private $_line; function __construct($line_number, $in_script_block) { @@ -288,6 +290,13 @@ class Xss_Security_Test_Frame { return $this->_is_safestring; } + function is_safe_html($new_val=NULL) { + if ($new_val !== NULL) { + $this->_is_safe_html = (bool) $new_val; + } + return $this->_is_safe_html; + } + function json_encode_called($new_val=NULL) { if ($new_val !== NULL) { $this->_json_encode_called = (bool) $new_val; diff --git a/modules/gallery/views/admin_block_welcome.html.php b/modules/gallery/views/admin_block_welcome.html.php index 38d2bd56..c6ccdbf3 100644 --- a/modules/gallery/views/admin_block_welcome.html.php +++ b/modules/gallery/views/admin_block_welcome.html.php @@ -5,16 +5,16 @@ diff --git a/modules/gallery/views/upgrader.html.php b/modules/gallery/views/upgrader.html.php index 37578855..ccc86da8 100644 --- a/modules/gallery/views/upgrader.html.php +++ b/modules/gallery/views/upgrader.html.php @@ -18,7 +18,7 @@

    Gallery is up to date.", - array("url" => url::site("albums/1"))) ?> + array("url" => SafeString::of_safe_html(url::site("albums/1")))) ?>

    diff --git a/modules/recaptcha/helpers/recaptcha.php b/modules/recaptcha/helpers/recaptcha.php index 501dd972..35d9febd 100644 --- a/modules/recaptcha/helpers/recaptcha.php +++ b/modules/recaptcha/helpers/recaptcha.php @@ -43,7 +43,7 @@ class recaptcha_Core { if (empty($public_key) || empty($private_key)) { site_status::warning( t("reCAPTCHA is not quite ready! Please configure the reCAPTCHA Keys", - array("url" => url::site("admin/recaptcha"))), + array("url" => SafeString::of_safe_html(url::site("admin/recaptcha")))), "recaptcha_config"); } else { site_status::clear("recaptcha_config"); diff --git a/modules/search/helpers/search.php b/modules/search/helpers/search.php index 355c4493..4be04039 100644 --- a/modules/search/helpers/search.php +++ b/modules/search/helpers/search.php @@ -58,7 +58,7 @@ class search_Core { if ($remaining) { site_status::warning( t('Your search index needs to be updated. Fix this now', - array("url" => url::site("admin/maintenance/start/search_task::update_index?csrf=__CSRF__"))), + array("url" => SafeString::of_safe_html(url::site("admin/maintenance/start/search_task::update_index?csrf=__CSRF__")))), "search_index_out_of_date"); } } diff --git a/modules/server_add/helpers/server_add.php b/modules/server_add/helpers/server_add.php index 74f51ad9..57afac12 100644 --- a/modules/server_add/helpers/server_add.php +++ b/modules/server_add/helpers/server_add.php @@ -25,7 +25,7 @@ class server_add_Core { if (empty($paths)) { site_status::warning( t("Server Add needs configuration. Configure it now!", - array("url" => url::site("admin/server_add"))), + array("url" => SafeString::of_safe_html(url::site("admin/server_add")))), "server_add_configuration"); } else { site_status::clear("server_add_configuration"); diff --git a/modules/user/views/reset_password.html.php b/modules/user/views/reset_password.html.php index 3dc7aebf..6fa92d54 100644 --- a/modules/user/views/reset_password.html.php +++ b/modules/user/views/reset_password.html.php @@ -9,7 +9,9 @@ $user->full_name ? $user->full_name : $user->name)) ?>

    - %site_url. If you made this request, you can confirm it by clicking this link. If you didn't request this password reset, it's ok to ignore this mail.", array("site_url" => url::base(false, "http"), "confirm_url" => $confirm_url)) ?> + %site_url. If you made this request, you can confirm it by clicking this link. If you didn't request this password reset, it's ok to ignore this mail.", + array("site_url" => SafeString::of_safe_html(url::base(false, "http")), + "confirm_url" => $confirm_url)) ?>

    diff --git a/system/helpers/request.php b/system/helpers/request.php index 15b8edfa..4203d0e5 100644 --- a/system/helpers/request.php +++ b/system/helpers/request.php @@ -30,7 +30,7 @@ class request_Core { // Set referrer $ref = $_SERVER['HTTP_REFERER']; - if (strpos($ref, (string) url::base(FALSE)) === 0) + if (strpos($ref, url::base(FALSE)) === 0) { // Remove the base URL from the referrer $ref = substr($ref, strlen(url::base(FALSE))); -- cgit v1.2.3 From 94c201f265c758fad38eb69c0a5878970119197a Mon Sep 17 00:00:00 2001 From: Andy Staudacher Date: Tue, 1 Sep 2009 01:17:39 -0700 Subject: XSS escape in form helper and forge where missing. --- modules/forge/libraries/Form_Checkbox.php | 2 +- modules/forge/libraries/Form_Checklist.php | 2 +- modules/forge/libraries/Form_Group.php | 2 +- system/helpers/form.php | 19 +++++++++++++------ 4 files changed, 16 insertions(+), 9 deletions(-) (limited to 'system/helpers') diff --git a/modules/forge/libraries/Form_Checkbox.php b/modules/forge/libraries/Form_Checkbox.php index b94fc438..aded4fdf 100644 --- a/modules/forge/libraries/Form_Checkbox.php +++ b/modules/forge/libraries/Form_Checkbox.php @@ -68,7 +68,7 @@ class Form_Checkbox_Core extends Form_Input { $label = ' '.ltrim($label); } - return ''; + return ''; } protected function load_value() diff --git a/modules/forge/libraries/Form_Checklist.php b/modules/forge/libraries/Form_Checklist.php index 99b455bd..4536d396 100644 --- a/modules/forge/libraries/Form_Checklist.php +++ b/modules/forge/libraries/Form_Checklist.php @@ -67,7 +67,7 @@ class Form_Checklist_Core extends Form_Input { $data['value'] = $val; $data['checked'] = $checked; - $checklist .= '
    • '.$nl; + $checklist .= '
    • '.$nl; } $checklist .= ''; diff --git a/modules/forge/libraries/Form_Group.php b/modules/forge/libraries/Form_Group.php index 29eff510..e0601321 100644 --- a/modules/forge/libraries/Form_Group.php +++ b/modules/forge/libraries/Form_Group.php @@ -57,7 +57,7 @@ class Form_Group_Core extends Forge { { if ($label = $this->data['label']) { - return $this->data['label']; + return html::purify($this->data['label']); } } else diff --git a/system/helpers/form.php b/system/helpers/form.php index ce8767c5..815eef84 100644 --- a/system/helpers/form.php +++ b/system/helpers/form.php @@ -283,15 +283,21 @@ class form_Core { // Inner key should always be a string $inner_key = (string) $inner_key; - $sel = in_array($inner_key, $selected) ? ' selected="selected"' : ''; - $input .= ''."\n"; + $attr = array('value' => $inner_key); + if (in_array($inner_key, $selected)) { + $attr['selected'] = 'selected'; + } + $input .= ''."\n"; } $input .= ''."\n"; } else { - $sel = in_array($key, $selected) ? ' selected="selected"' : ''; - $input .= ''."\n"; + $attr = array('value' => $key); + if (in_array($key, $selected)) { + $attr['selected'] = 'selected'; + } + $input .= ''."\n"; } } $input .= ''; @@ -410,8 +416,9 @@ class form_Core { { $value = arr::remove('value', $data); } + // $value must be ::purify - return ''.$value.''; + return ''.html::purify($value).''; } /** @@ -455,7 +462,7 @@ class form_Core { $text = ucwords(inflector::humanize($data['for'])); } - return ''.$text.''; + return ''.html::purify($text).''; } /** -- cgit v1.2.3 From ca1947a1ce9d3f70df88c6a5f6260923c5f035c1 Mon Sep 17 00:00:00 2001 From: Bharat Mediratta Date: Fri, 4 Sep 2009 20:15:21 -0700 Subject: Updated upstream to r4554 --- system/helpers/url.php | 9 ++++++--- system/libraries/Session.php | 9 +++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) (limited to 'system/helpers') diff --git a/system/helpers/url.php b/system/helpers/url.php index f3d0ec8b..56f6db4b 100644 --- a/system/helpers/url.php +++ b/system/helpers/url.php @@ -2,7 +2,7 @@ /** * URL helper class. * - * $Id: url.php 4029 2009-03-03 12:39:32Z Shadowhand $ + * $Id: url.php 4479 2009-07-23 04:51:22Z ixmatus $ * * @package Core * @author Kohana Team @@ -15,11 +15,14 @@ class url_Core { * Fetches the current URI. * * @param boolean include the query string + * @param boolean include the suffix * @return string */ - public static function current($qs = FALSE) + public static function current($qs = FALSE, $suffix = FALSE) { - return ($qs === TRUE) ? Router::$complete_uri : Router::$current_uri; + $uri = ($qs === TRUE) ? Router::$complete_uri : Router::$current_uri; + + return ($suffix === TRUE) ? $uri.Kohana::config('core.url_suffix') : $uri; } /** diff --git a/system/libraries/Session.php b/system/libraries/Session.php index 670ee6a6..51acce00 100644 --- a/system/libraries/Session.php +++ b/system/libraries/Session.php @@ -2,7 +2,7 @@ /** * Session library. * - * $Id: Session.php 4433 2009-07-01 03:44:20Z kiall $ + * $Id: Session.php 4493 2009-07-27 20:05:41Z ixmatus $ * * @package Core * @author Kohana Team @@ -43,11 +43,16 @@ class Session_Core { return Session::$instance; } + + /** + * Be sure to block the use of __clone. + */ + private function __clone(){} /** * On first session instance creation, sets up the driver and creates session. */ - public function __construct() + protected function __construct() { $this->input = Input::instance(); -- cgit v1.2.3