Skip to content
Permalink
Browse files Browse the repository at this point in the history
Better XSS protection
* Add HTMLPurifier library (LGPL)
* Add helper functions to html helper
* Set default encoding header to UTF-8
* Make sure the doctype is the same everywhere (admin/members/frontend)
* Remove use of strip_tags() and htmlspecialchars()
* Replace vanilla htmlentities with html::escape() - make sure no one forgets the UTF-8
* Remove _csv_text() fn - no longer used and was using strip_tags()
  • Loading branch information
rjmackay committed Apr 9, 2013
1 parent 765a3ee commit 593719f
Show file tree
Hide file tree
Showing 409 changed files with 27,911 additions and 180 deletions.
2 changes: 1 addition & 1 deletion application/controllers/admin/reports.php
Expand Up @@ -102,7 +102,7 @@ public function index($page = 1)
$order_field = 'date'; $sort = 'DESC';
if (isset($_GET['order']))
{
$order_field = htmlentities($_GET['order']);
$order_field = html::escape($_GET['order']);
}
if (isset($_GET['sort']))
{
Expand Down
4 changes: 2 additions & 2 deletions application/controllers/feed.php
Expand Up @@ -105,12 +105,12 @@ public function index($feedtype = 'rss2')

//header("Content-Type: text/xml; charset=utf-8");
$view = new View('feed/'.$feedtype);
$view->feed_title = htmlspecialchars(Kohana::config('settings.site_name'));
$view->feed_title = Kohana::config('settings.site_name');
$view->site_url = $site_url;
$view->georss = 1; // this adds georss namespace in the feed
$view->feed_url = $site_url.$feedpath;
$view->feed_date = gmdate("D, d M Y H:i:s T", time());
$view->feed_description = htmlspecialchars(Kohana::lang('ui_admin.incident_feed').' '.Kohana::config('settings.site_name'));
$view->feed_description = Kohana::lang('ui_admin.incident_feed').' '.Kohana::config('settings.site_name');
$view->items = $feed_items;
$view->render(TRUE);
}
Expand Down
8 changes: 3 additions & 5 deletions application/controllers/json.php
Expand Up @@ -685,11 +685,9 @@ protected function get_geometry($incident_id, $incident_title, $incident_date, $
$title = ($item->geometry_label) ? $item->geometry_label : $incident_title;
$item_name = $this->get_title($title, $incident_link);

$fillcolor = ($item->geometry_color) ?
utf8tohtml::convert($item->geometry_color,TRUE) : "ffcc66";
$fillcolor = ($item->geometry_color) ? $item->geometry_color : "ffcc66";

$strokecolor = ($item->geometry_color) ?
utf8tohtml::convert($item->geometry_color,TRUE) : "CC0000";
$strokecolor = ($item->geometry_color) ? $item->geometry_color : "CC0000";

$strokewidth = ($item->geometry_strokewidth) ? $item->geometry_strokewidth : "3";

Expand All @@ -699,7 +697,7 @@ protected function get_geometry($incident_id, $incident_title, $incident_date, $
'id' => $incident_id,
'feature_id' => $item->id,
'name' => $item_name,
'description' => utf8tohtml::convert($item->geometry_comment,TRUE),
'description' => $item->geometry_comment,
'color' => $fillcolor,
'icon' => '',
'strokecolor' => $strokecolor,
Expand Down
6 changes: 0 additions & 6 deletions application/controllers/members/reports.php
Expand Up @@ -786,10 +786,4 @@ private function _get_searchstring($keyword_raw)
return "1=1";
}
}

private function _csv_text($text)
{
$text = stripslashes(htmlspecialchars($text));
return $text;
}
}
6 changes: 3 additions & 3 deletions application/controllers/reports.php
Expand Up @@ -547,10 +547,10 @@ public function view($id = FALSE)
}
else
{
$comment->comment_author = strip_tags($post->comment_author);
$comment->comment_email = strip_tags($post->comment_email);
$comment->comment_author = html::strip_tags($post->comment_author, FALSE);
$comment->comment_email = html::strip_tags($post->comment_email, FALSE);
}
$comment->comment_description = strip_tags($post->comment_description);
$comment->comment_description = html::strip_tags($post->comment_description, FALSE);
$comment->comment_ip = $_SERVER['REMOTE_ADDR'];
$comment->comment_date = date("Y-m-d H:i:s",time());

Expand Down
1 change: 1 addition & 0 deletions application/controllers/scheduler/s_alerts.php
Expand Up @@ -91,6 +91,7 @@ public function index()
// Convert HTML to Text
$incident_description = $incident->incident_description;
$incident_url = url::site().'reports/view/'.$incident->id;
$incident_description = html::clean($incident_description);
$html2text = new Html2Text($incident_description);
$incident_description = $html2text->get_text();

Expand Down
78 changes: 78 additions & 0 deletions application/helpers/MY_html.php
@@ -0,0 +1,78 @@
<?php defined('SYSPATH') OR die('No direct access allowed.');
/**
* HTML helper class.
*
* LICENSE: This source file is subject to LGPL license
* that is available through the world-wide-web at the following URI:
* http://www.gnu.org/copyleft/lesser.html
* @author Ushahidi Team <team@ushahidi.com>
* @package Ushahidi - http://source.ushahididev.com
* @module File Helper
* @copyright Ushahidi - http://www.ushahidi.com
* @license http://www.gnu.org/copyleft/lesser.html GNU Lesser General Public License (LGPL)
*/
class html extends html_Core {

/**
* Helper function for easy use of HTMLPurifier
*/
public function clean($input)
{
require_once APPPATH.'libraries/htmlpurifier/HTMLPurifier.auto.php';

$config = HTMLPurifier_Config::createDefault();
// Defaults to UTF-8
// $config->set('Core.Encoding', 'UTF-8');
// $config->set('HTML.Doctype', 'XHTML 1.0 Transitional');
$config->set('Core.EnableIDNA', TRUE);
$config->set('HTML.Allowed', "a[href|title],p,img[src|alt],br,b,u,strong,em,i");
// Allow some basic iframes
$config->set('HTML.SafeIframe', true);
$config->set('URI.SafeIframeRegexp',
'%^http://(www.youtube.com/embed/|player.vimeo.com/video/|w.soundcloud.com/player)%'
);
$config->set('Filter.YouTube', true);
$purifier = new HTMLPurifier($config);
$clean_html = $purifier->purify($input);

return $clean_html;
}

/**
* Helper function to clean and escape plaintext before display
*
* This should be used to strip tags and then escape html entities, etc.
*/
public function strip_tags($input, $encode = TRUE)
{
require_once APPPATH.'libraries/htmlpurifier/HTMLPurifier.auto.php';

$config = HTMLPurifier_Config::createDefault();
// Defaults to UTF-8
// $config->set('Core.Encoding', 'UTF-8');
// $config->set('HTML.Doctype', 'XHTML 1.0 Transitional');
$config->set('Core.EnableIDNA', TRUE);
$config->set('HTML.Allowed', "");

$purifier = new HTMLPurifier($config);
$clean_html = $purifier->purify($input);

return $encode ? self::escape($clean_html) : $clean_html;
}

/**
* Helper function to escape plaintext before display
*
* This should be used to escape html entities, etc.
*/
public function escape($input)
{
// Ensure we have valid correctly encoded string..
// http://stackoverflow.com/questions/1412239/why-call-mb-convert-encoding-to-sanitize-text
$input = mb_convert_encoding($input, "UTF-8", "UTF-8");
// why are we using html entities? this -> http://stackoverflow.com/a/110576/992171
return htmlentities($input, ENT_QUOTES, 'UTF-8');
}

}

10 changes: 5 additions & 5 deletions application/helpers/category.php
Expand Up @@ -194,8 +194,8 @@ public static function get_category_tree_data($count = FALSE, $include_hidden =

$category_data[$category->id] = array(
'category_id' => $category->id,
'category_title' => htmlentities(Category_Lang_Model::category_title($category->id), ENT_QUOTES, "UTF-8"),
'category_description' => htmlentities(Category_Lang_Model::category_description($category->id), ENT_QUOTES, "UTF-8"),
'category_title' => html::escape(Category_Lang_Model::category_title($category->id)),
'category_description' => html::escape(Category_Lang_Model::category_description($category->id)),
'category_color' => $category->category_color,
'category_image' => $category->category_image,
'children' => $children,
Expand Down Expand Up @@ -226,8 +226,8 @@ public static function get_category_tree_data($count = FALSE, $include_hidden =
// Add children
$category_data[$category->parent_id]['children'][$category->id] = array(
'category_id' => $category->id,
'category_title' => htmlentities(Category_Lang_Model::category_title($category->id), ENT_QUOTES, "UTF-8"),
'category_description' => htmlentities(Category_Lang_Model::category_description($category->id), ENT_QUOTES, "UTF-8"),
'category_title' => html::escape(Category_Lang_Model::category_title($category->id)),
'category_description' => html::escape(Category_Lang_Model::category_description($category->id)),
'parent_id' => $category->parent_id,
'category_color' => $category->category_color,
'category_image' => $category->category_image,
Expand Down Expand Up @@ -264,7 +264,7 @@ private static function _generate_treeview_html($category_data)
$tree_html .= "<li".$category_class.">"
. "<a href=\"#\" class=\"cat_selected\" id=\"filter_link_cat_".$id."\" title=\"{$category['category_description']}\">"
. "<span class=\"item-swatch\" style=\"background-color: #".$category['category_color']."\">$category_image</span>"
. "<span class=\"item-title\">".strip_tags($category['category_title'])."</span>"
. "<span class=\"item-title\">".html::strip_tags($category['category_title'])."</span>"
. "<span class=\"item-count\">".$category['report_count']."</span>"
. "</a></li>";

Expand Down
48 changes: 0 additions & 48 deletions application/helpers/utf8tohtml.php

This file was deleted.

7 changes: 2 additions & 5 deletions application/hooks/actions.php
Expand Up @@ -862,11 +862,8 @@ public function __response_create_report($vars)
// If this is a feed item
elseif (isset($this->data->item_title))
{
$incident_title = strip_tags(html_entity_decode(html_entity_decode($this->data->item_title, ENT_QUOTES)));
$incident_description = strip_tags(
// @todo place with real html sanitizing
str_ireplace(array('<p>','</p>','<br>','<br />','<br/>'), "\n", html_entity_decode($this->data->item_description, ENT_QUOTES))
);
$incident_title = html::strip_tags(html_entity_decode(html_entity_decode($this->data->item_title, ENT_QUOTES)));
$incident_description = html::clean(html_entity_decode($this->data->item_description, ENT_QUOTES));
$incident_date = $this->data->item_date;
}

Expand Down
7 changes: 4 additions & 3 deletions application/libraries/Imap.php
Expand Up @@ -183,16 +183,17 @@ public function get_messages($search_criteria="UNSEEN",

// This isn't the perfect solution but windows-1256 encoding doesn't work with mb_detect_encoding()
// so if it doesn't return an encoding, lets assume it's arabic. (sucks)
if(mb_detect_encoding($body, 'auto', true) == '')
if(mb_detect_encoding($body, 'auto', TRUE) == '')
{
$body = iconv("windows-1256", "UTF-8", $body);
}

// Convert to valid UTF8
$detected_encoding = mb_detect_encoding($body, "auto");
if($detected_encoding == 'ASCII') $detected_encoding = 'iso-8859-1';
$body = htmlentities($body,NULL,$detected_encoding);
$subject = htmlentities(strip_tags($subject),NULL,'UTF-8');
$body = mb_convert_encoding($body, $detected_encoding, 'UTF-8');
$body = html::escape($body);
$subject = html::strip_tags($subject);

array_push($messages, array('message_id' => $message_id,
'date' => $date,
Expand Down
3 changes: 3 additions & 0 deletions application/libraries/MY_Controller.php
Expand Up @@ -77,5 +77,8 @@ public function __construct()
url::redirect('login');
}
}

// Set default content-type header
header('Content-type: text/html; charset=UTF-8');
}
}
10 changes: 5 additions & 5 deletions application/libraries/VideoEmbed.php
Expand Up @@ -160,7 +160,7 @@ public function embed($raw, $auto = FALSE, $echo = TRUE)
$you_auto = ($auto) ? "&autoplay=1" : "";

$output = '<iframe id="ytplayer" type="text/html" width="320" height="265" '
. 'src="http://www.youtube.com/embed/'.htmlentities($code, ENT_QUOTES, "UTF-8").'?origin='.urlencode(url::base()).htmlentities($you_auto, ENT_QUOTES, "UTF-8").'" '
. 'src="http://www.youtube.com/embed/'.html::escape($code).'?origin='.urlencode(url::base()).html::escape($you_auto).'" '
. 'frameborder="0"></iframe>';
break;

Expand All @@ -169,29 +169,29 @@ public function embed($raw, $auto = FALSE, $echo = TRUE)
$google_auto = ($auto) ? "&autoPlay=true" : "";

$output = "<embed style='width:320px; height:265px;' id='VideoPlayback' type='application/x-shockwave-flash'"
. " src='http://video.google.com/googleplayer.swf?docId=-".htmlentities($code.$google_auto, ENT_QUOTES, "UTF-8")."&hl=en' flashvars=''>"
. " src='http://video.google.com/googleplayer.swf?docId=-".html::escape($code.$google_auto)."&hl=en' flashvars=''>"
. "</embed>";
break;

case "metacafe":
// Sanitize input
$code = strrev(trim(strrev($code), "/"));

$output = "<embed src='http://www.metacafe.com/fplayer/".htmlentities($code, ENT_QUOTES, "UTF-8").".swf'"
$output = "<embed src='http://www.metacafe.com/fplayer/".html::escape($code).".swf'"
. " width='320' height='265' wmode='transparent' pluginspage='http://get.adobe.com/flashplayer/'"
. " type='application/x-shockwave-flash'> "
. "</embed>";
break;

case "dotsub":
$output = "<iframe src='http://dotsub.com/media/".htmlentities($code, ENT_QUOTES, "UTF-8")."' frameborder='0' width='320' height='500'></iframe>";
$output = "<iframe src='http://dotsub.com/media/".html::escape($code)."' frameborder='0' width='320' height='500'></iframe>";

break;

case "vimeo":
$vimeo_auto = ($auto) ? "?autoplay=1" : "";

$output = '<iframe src="http://player.vimeo.com/video/'.htmlentities($code.$vimeo_auto, ENT_QUOTES, "UTF-8").'" width="320" height="265" frameborder="0">'
$output = '<iframe src="http://player.vimeo.com/video/'.html::escape($code.$vimeo_auto).'" width="320" height="265" frameborder="0">'
. '</iframe>';
break;
}
Expand Down
4 changes: 2 additions & 2 deletions application/libraries/XMLImporter.php
Expand Up @@ -313,7 +313,7 @@ public function import_categories($categories)

// Also add it to the array of categories added during import
$this->categories_added[] = $new_category->id;
$this->notices[] = Kohana::lang('import.new_category').htmlspecialchars($cat_title);
$this->notices[] = Kohana::lang('import.new_category').html::escape($cat_title);
}

/* Category Translations */
Expand Down Expand Up @@ -687,7 +687,7 @@ public function import_reports($reports)
// If report date is not in the required format
if ( ! strtotime($report_date))
{
$this->errors[] = Kohana::lang('import.incident_date').$this->totalreports.': '.htmlspecialchars($report_date);
$this->errors[] = Kohana::lang('import.incident_date').$this->totalreports.': '.html::escape($report_date);
}

// Report title and date(in correct format) both provided, proceed
Expand Down
10 changes: 5 additions & 5 deletions application/libraries/api/MY_Comments_Api_Object.php
Expand Up @@ -695,11 +695,11 @@ private function _add_comment()
}

$comment = new Comment_Model();
$comment->incident_id = strip_tags($incident_id);
$comment->checkin_id = strip_tags($checkin_id);
$comment->comment_author = strip_tags($comment_author);
$comment->comment_description = strip_tags($post->comment_description);
$comment->comment_email = strip_tags($comment_email);
$comment->incident_id = intval($incident_id);
$comment->checkin_id = intval($checkin_id);
$comment->comment_author = html::strip_tags($comment_author, FALSE);
$comment->comment_description = html::strip_tags($post->comment_description, FALSE);
$comment->comment_email = html::strip_tags($comment_email, FALSE);
$comment->comment_ip = $_SERVER['REMOTE_ADDR'];
$comment->comment_date = date("Y-m-d H:i:s", time());

Expand Down
9 changes: 9 additions & 0 deletions application/libraries/htmlpurifier/CREDITS
@@ -0,0 +1,9 @@

CREDITS

Almost everything written by Edward Z. Yang (Ambush Commander). Lots of thanks
to the DevNetwork Community for their help (see docs/ref-devnetwork.html for
more details), Feyd especially (namely IPv6 and optimization). Thanks to RSnake
for letting me package his fantastic XSS cheatsheet for a smoketest.

vim: et sw=4 sts=4
11 changes: 11 additions & 0 deletions application/libraries/htmlpurifier/HTMLPurifier.auto.php
@@ -0,0 +1,11 @@
<?php

/**
* This is a stub include that automatically configures the include path.
*/

set_include_path(dirname(__FILE__) . PATH_SEPARATOR . get_include_path() );
require_once 'HTMLPurifier/Bootstrap.php';
require_once 'HTMLPurifier.autoload.php';

// vim: et sw=4 sts=4

0 comments on commit 593719f

Please sign in to comment.