Skip to content

Commit af021cb

Browse files
committed
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
1 parent 2849bb0 commit af021cb

File tree

4 files changed

+22
-4
lines changed

4 files changed

+22
-4
lines changed

cake/libs/controller/controller.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,6 +1042,7 @@ function paginate($object = null, $scope = array(), $whitelist = array()) {
10421042
} elseif (intval($page) < 1) {
10431043
$options['page'] = $page = 1;
10441044
}
1045+
$page = $options['page'] = (integer)$page;
10451046

10461047
if (method_exists($object, 'paginate')) {
10471048
$results = $object->paginate($conditions, $fields, $order, $limit, $page, $recursive, $extra);

cake/libs/view/helper.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ function loadConfig($name = 'tags') {
175175
* @return string Full translated URL with base path.
176176
*/
177177
function url($url = null, $full = false) {
178-
return Router::url($url, array('full' => $full, 'escape' => true));
178+
return h(Router::url($url, $full));
179179
}
180180
/**
181181
* Checks if a file exists when theme is used, if no file is found default location is returned

cake/tests/cases/libs/controller/controller.test.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -503,10 +503,12 @@ function testPaginate() {
503503
$results = Set::extract($Controller->paginate('ControllerPost'), '{n}.ControllerPost.id');
504504
$this->assertEqual($Controller->ControllerPost->lastQuery['order'][0], array('ControllerPost.author_id' => 'asc'));
505505
$this->assertEqual($results, array(1, 3, 2));
506-
507-
$Controller->passedArgs = array('page' => '" onclick="alert(\'xss\');">');
506+
507+
$Controller->passedArgs = array('page' => '1 " onclick="alert(\'xss\');">');
508+
$Controller->paginate = array('limit' => 1);
508509
$Controller->paginate('ControllerPost');
509-
$this->assertEqual($Controller->params['paging']['ControllerPost']['page'], 1, 'XSS exploit opened %s');
510+
$this->assertIdentical($Controller->params['paging']['ControllerPost']['page'], 1, 'XSS exploit opened %s');
511+
$this->assertIdentical($Controller->params['paging']['ControllerPost']['options']['page'], 1, 'XSS exploit opened %s');
510512
}
511513
/**
512514
* testPaginateExtraParams method

cake/tests/cases/libs/view/helper.test.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,21 @@ function testValue() {
347347
$result = $this->Helper->value('Post.2.created.year');
348348
$this->assertEqual($result, '2008');
349349
}
350+
/**
351+
* Ensure HTML escaping of url params. So link addresses are valid and not exploited
352+
*
353+
* @return void
354+
**/
355+
function testUrlConversion() {
356+
$result = $this->Helper->url('/controller/action/1');
357+
$this->assertEqual($result, '/controller/action/1');
358+
359+
$result = $this->Helper->url('/controller/action/1?one=1&two=2');
360+
$this->assertEqual($result, '/controller/action/1?one=1&amp;two=2');
361+
362+
$result = $this->Helper->url(array('controller' => 'posts', 'action' => 'index', 'page' => '1" onclick="alert(\'XSS\');"'));
363+
$this->assertEqual($result, "/posts/index/page:1&quot; onclick=&quot;alert(&#039;XSS&#039;);&quot;");
364+
}
350365
/**
351366
* testFieldsWithSameName method
352367
*

0 commit comments

Comments
 (0)