Skip to content

Commit

Permalink
Escape urls to avoid xss
Browse files Browse the repository at this point in the history
  • Loading branch information
ceeram committed Aug 25, 2017
1 parent 91274dd commit 0c88f63
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/View/Helper/HtmlHelper.php
Expand Up @@ -826,7 +826,7 @@ protected function _prepareCrumbs($startText, $escape = true)
* - `fullBase` If true the src attribute will get a full address for the image file.
* - `plugin` False value will prevent parsing path as a plugin
*
* @param string $path Path to the image file, relative to the app/webroot/img/ directory.
* @param string|array $path Path to the image file, relative to the app/webroot/img/ directory.
* @param array $options Array of HTML attributes. See above for special options.
* @return string completed img tag
* @link https://book.cakephp.org/3.0/en/views/helpers/html.html#linking-to-images
Expand Down
2 changes: 1 addition & 1 deletion src/View/Helper/UrlHelper.php
Expand Up @@ -144,7 +144,7 @@ public function assetUrl($path, array $options = [])
return $this->build($path, !empty($options['fullBase']));
}
if (strpos($path, '://') !== false || preg_match('/^[a-z]+:/i', $path)) {
return $path;
return ltrim($this->build($path), '/');
}
if (!array_key_exists('plugin', $options) || $options['plugin'] !== false) {
list($plugin, $path) = $this->_View->pluginSplit($path, false);
Expand Down
3 changes: 3 additions & 0 deletions src/View/StringTemplate.php
Expand Up @@ -315,6 +315,9 @@ protected function _formatAttribute($key, $value, $escape = true)
}
$truthy = [1, '1', true, 'true', $key];
$isMinimized = isset($this->_compactAttributes[$key]);
if (!preg_match('/\A(\w|[.-])+\z/', $key)) {
$key = h($key);
}
if ($isMinimized && in_array($value, $truthy, true)) {
return "$key=\"$key\"";
}
Expand Down
39 changes: 39 additions & 0 deletions tests/TestCase/View/Helper/HtmlHelperTest.php
Expand Up @@ -347,6 +347,11 @@ public function testImageTag()
$result = $this->Html->image('cid:cakephp_logo');
$expected = ['img' => ['src' => 'cid:cakephp_logo', 'alt' => '']];
$this->assertHtml($expected, $result);

$result = $this->Html->image('x:"><script>alert(1)</script>');
$expected = ['img' => ['src' => 'x:&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;', 'alt' => '']];

$this->assertHtml($expected, $result);
}

/**
Expand Down Expand Up @@ -562,6 +567,10 @@ public function testCssLink()
$expected['link']['href'] = 'css/screen.css?with=param&amp;other=param';
$this->assertHtml($expected, $result);

$result = $this->Html->css('x:"><script>alert(1)</script>');
$expected['link']['href'] = 'x:&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;';
$this->assertHtml($expected, $result);

$result = $this->Html->css('http://whatever.com/screen.css?1234');
$expected['link']['href'] = 'preg:/http:\/\/.*\/screen\.css\?1234/';
$this->assertHtml($expected, $result);
Expand Down Expand Up @@ -904,6 +913,12 @@ public function testScript()
];
$this->assertHtml($expected, $result);

$result = $this->Html->script('x:"><script>alert(1)</script>');
$expected = [
'script' => ['src' => 'x:&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;']
];
$this->assertHtml($expected, $result);

$result = $this->Html->script('foo2', ['pathPrefix' => '/my/custom/path/']);
$expected = [
'script' => ['src' => '/my/custom/path/foo2.js']
Expand Down Expand Up @@ -1716,6 +1731,24 @@ public function testMetaIcon()
];
$this->assertHtml($expected, $result);

$result = $this->Html->meta('icon', 'x:"><script>alert(1)</script>');
$url = 'x:&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;';
$expected = [
'link' => [
'href' => $url,
'type' => 'image/x-icon',
'rel' => 'icon'
],
[
'link' => [
'href' => $url,
'type' => 'image/x-icon',
'rel' => 'shortcut icon'
]
]
];
$this->assertHtml($expected, $result);

$this->Html->request->webroot = '/testing/';
$result = $this->Html->meta('icon');
$expected = [
Expand Down Expand Up @@ -1956,6 +1989,12 @@ public function testDiv()
$result = $this->Html->div('class-name', '<text>', ['escape' => true]);
$expected = ['div' => ['class' => 'class-name'], '&lt;text&gt;', '/div'];
$this->assertHtml($expected, $result);

$evilKey = "><script>alert(1)</script>";
$options = [$evilKey => 'some value'];
$result = $this->Html->div('class-name', '', $options);
$expected = '<div &gt;&lt;script&gt;alert(1)&lt;/script&gt;="some value" class="class-name"></div>';
$this->assertEquals($expected, $result);
}

/**
Expand Down
3 changes: 3 additions & 0 deletions tests/TestCase/View/Helper/UrlHelperTest.php
Expand Up @@ -198,6 +198,9 @@ public function testAssetUrl()
$result = $this->Helper->assetUrl('foo.jpg?one=two&three=four');
$this->assertEquals('foo.jpg?one=two&amp;three=four', $result);

$result = $this->Helper->assetUrl('x:"><script>alert(1)</script>');
$this->assertEquals('x:&quot;&gt;&lt;script&gt;alert(1)&lt;/script&gt;', $result);

$result = $this->Helper->assetUrl('dir/big+tall/image', ['ext' => '.jpg']);
$this->assertEquals('dir/big%2Btall/image.jpg', $result);
}
Expand Down
9 changes: 9 additions & 0 deletions tests/TestCase/View/StringTemplateTest.php
Expand Up @@ -264,6 +264,15 @@ public function testFormatAttributes()
' data-hero="&lt;batman&gt;"',
$result
);

$evilKey = "><script>alert(1)</script>";
$attrs = [$evilKey => 'some value'];

$result = $this->template->formatAttributes($attrs);
$this->assertEquals(
' &gt;&lt;script&gt;alert(1)&lt;/script&gt;="some value"',
$result
);
}

/**
Expand Down

0 comments on commit 0c88f63

Please sign in to comment.