From af021cb4b230708938be04781bd191872f13bfa3 Mon Sep 17 00:00:00 2001
From: mark_story <mark@mark-story.com>
Date: Wed, 25 Feb 2009 19:51:41 +0000
Subject: [PATCH] Adding Html entity conversion to all urls generated by
 helpers, fixing potential for merged passedArgs to create xss vectors. Adding
 integer cast in paginate() to page param. Tests added/updated. Fixes #6134

git-svn-id: https://svn.cakephp.org/repo/branches/1.2.x.x@8061 3807eeeb-6ff5-0310-8944-8be069107fe0
---
 cake/libs/controller/controller.php               |  1 +
 cake/libs/view/helper.php                         |  2 +-
 .../cases/libs/controller/controller.test.php     |  8 +++++---
 cake/tests/cases/libs/view/helper.test.php        | 15 +++++++++++++++
 4 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/cake/libs/controller/controller.php b/cake/libs/controller/controller.php
index 4a85cac6ce0..3834d970c44 100644
--- a/cake/libs/controller/controller.php
+++ b/cake/libs/controller/controller.php
@@ -1042,6 +1042,7 @@ function paginate($object = null, $scope = array(), $whitelist = array()) {
 		} elseif (intval($page) < 1) {
 			$options['page'] = $page = 1;
 		}
+		$page = $options['page'] = (integer)$page;
 
 		if (method_exists($object, 'paginate')) {
 			$results = $object->paginate($conditions, $fields, $order, $limit, $page, $recursive, $extra);
diff --git a/cake/libs/view/helper.php b/cake/libs/view/helper.php
index b365d7d744e..fc12c7ac2d0 100644
--- a/cake/libs/view/helper.php
+++ b/cake/libs/view/helper.php
@@ -175,7 +175,7 @@ function loadConfig($name = 'tags') {
  * @return string  Full translated URL with base path.
  */
 	function url($url = null, $full = false) {
-		return Router::url($url, array('full' => $full, 'escape' => true));
+		return h(Router::url($url, $full));
 	}
 /**
  * Checks if a file exists when theme is used, if no file is found default location is returned
diff --git a/cake/tests/cases/libs/controller/controller.test.php b/cake/tests/cases/libs/controller/controller.test.php
index 3730ad253a0..e0bbfeda939 100644
--- a/cake/tests/cases/libs/controller/controller.test.php
+++ b/cake/tests/cases/libs/controller/controller.test.php
@@ -503,10 +503,12 @@ function testPaginate() {
 		$results = Set::extract($Controller->paginate('ControllerPost'), '{n}.ControllerPost.id');
 		$this->assertEqual($Controller->ControllerPost->lastQuery['order'][0], array('ControllerPost.author_id' => 'asc'));
 		$this->assertEqual($results, array(1, 3, 2));
-		
-		$Controller->passedArgs = array('page' => '" onclick="alert(\'xss\');">');
+
+		$Controller->passedArgs = array('page' => '1 " onclick="alert(\'xss\');">');
+		$Controller->paginate = array('limit' => 1);
 		$Controller->paginate('ControllerPost');
-		$this->assertEqual($Controller->params['paging']['ControllerPost']['page'], 1, 'XSS exploit opened %s');
+		$this->assertIdentical($Controller->params['paging']['ControllerPost']['page'], 1, 'XSS exploit opened %s');
+		$this->assertIdentical($Controller->params['paging']['ControllerPost']['options']['page'], 1, 'XSS exploit opened %s');
 	}
 /**
  * testPaginateExtraParams method
diff --git a/cake/tests/cases/libs/view/helper.test.php b/cake/tests/cases/libs/view/helper.test.php
index 922719c9935..ad4a9b6db20 100644
--- a/cake/tests/cases/libs/view/helper.test.php
+++ b/cake/tests/cases/libs/view/helper.test.php
@@ -347,6 +347,21 @@ function testValue() {
 		$result = $this->Helper->value('Post.2.created.year');
 		$this->assertEqual($result, '2008');
 	}
+/**
+ * Ensure HTML escaping of url params.  So link addresses are valid and not exploited
+ *
+ * @return void
+ **/
+	function testUrlConversion() {
+		$result = $this->Helper->url('/controller/action/1');
+		$this->assertEqual($result, '/controller/action/1');
+		
+		$result = $this->Helper->url('/controller/action/1?one=1&two=2');
+		$this->assertEqual($result, '/controller/action/1?one=1&amp;two=2');
+		
+		$result = $this->Helper->url(array('controller' => 'posts', 'action' => 'index', 'page' => '1" onclick="alert(\'XSS\');"'));
+		$this->assertEqual($result, "/posts/index/page:1&quot; onclick=&quot;alert(&#039;XSS&#039;);&quot;");
+	}
 /**
  * testFieldsWithSameName method
  *