Skip to content
This repository has been archived by the owner on May 1, 2019. It is now read-only.

Commit

Permalink
Merge pull request #313 from ins0/fix/xss
Browse files Browse the repository at this point in the history
[WIP] Fix/Added escape helper to view output
  • Loading branch information
GeeH committed Jan 23, 2015
2 parents 63a545d + 4129af5 commit 4ad4a78
Show file tree
Hide file tree
Showing 20 changed files with 383 additions and 181 deletions.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
"evandotpro/edp-module-layouts": "1.0.*",
"zf-commons/zfc-user": "1.0.*",
"zfcampus/zf-development-mode": "~2.0",
"socalnick/scn-social-auth": "1.14.1"
"socalnick/scn-social-auth": "1.14.1",
"ezyang/htmlpurifier": "4.6.*"
},
"require-dev": {
"phpmd/phpmd": "~2.1",
Expand Down
180 changes: 111 additions & 69 deletions composer.lock

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion module/Application/config/module.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

use Application\Service;
use Application\View;

return array(
'router' => array(
Expand Down Expand Up @@ -90,6 +91,7 @@
'factories' => array(
'translator' => 'Zend\I18n\Translator\TranslatorServiceFactory',
Service\RepositoryRetriever::class => Service\RepositoryRetrieverFactory::class,
\HTMLPurifier::class => Service\HtmlPurifierFactory::class,
),
),
'translator' => array(
Expand Down Expand Up @@ -134,11 +136,12 @@
),
'view_helpers' => array(
'factories' => array(
'sanitizeHtml' => View\Helper\SanitizeHtmlFactory::class,
'flashMessenger' => function ($sm) {
$sm = $sm->getServiceLocator();
$plugin = $sm->get('ControllerPluginManager')->get('flashMessenger');

$helper = new Application\View\Helper\FlashMessenger($plugin);
$helper = new View\Helper\FlashMessenger($plugin);
return $helper;
}
)
Expand Down
26 changes: 26 additions & 0 deletions module/Application/src/Application/Service/HtmlPurifierFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Application\Service;

use Zend\ServiceManager\FactoryInterface;
use Zend\ServiceManager\ServiceLocatorInterface;

class HtmlPurifierFactory implements FactoryInterface
{
/**
* {@inheritDoc}
*
* @return \HTMLPurifier
*/
public function createService(ServiceLocatorInterface $serviceLocator)
{
$config = $serviceLocator->get('Config');

$options = [];
if (isset($config['htmlpurifier'])) {
$options = $config['htmlpurifier'];
}

return new \HTMLPurifier($options);
}
}
20 changes: 20 additions & 0 deletions module/Application/src/Application/View/Helper/SanitizeHtml.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace Application\View\Helper;

use Zend\View\Helper\AbstractHelper;

class SanitizeHtml extends AbstractHelper
{
private $htmlPurifier;

public function __construct(\HTMLPurifier $htmlPurifier)
{
$this->htmlPurifier = $htmlPurifier;
}

public function __invoke($dirtyHtml)
{
return $this->htmlPurifier->purify($dirtyHtml);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

namespace Application\View\Helper;

use Zend\ServiceManager\FactoryInterface;
use Zend\ServiceManager\ServiceLocatorInterface;

class SanitizeHtmlFactory implements FactoryInterface
{
/**
* {@inheritDoc}
*
* @return SanitizeHtml
*/
public function createService(ServiceLocatorInterface $pluginManager)
{
/** @var \Zend\View\HelperPluginManager $pluginManager */
$serviceLocator = $pluginManager->getServiceLocator();
$htmlPurifier = $serviceLocator->get(\HTMLPurifier::class);

return new SanitizeHtml($htmlPurifier);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace ApplicationTest\Integration\Service;

use ApplicationTest\Integration\Util\Bootstrap;
use PHPUnit_Framework_TestCase;

class HtmlPurifierTest extends PHPUnit_Framework_TestCase
{
public function testServiceCanBeRetrieved()
{
$serviceManager = Bootstrap::getServiceManager();

$this->assertInstanceOf(
\HTMLPurifier::class,
$serviceManager->get(\HTMLPurifier::class)
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace ApplicationTest\Integration\View\Helper;

use Application\View\Helper\SanitizeHtml;
use ApplicationTest\Integration\Util\Bootstrap;
use PHPUnit_Framework_TestCase;

class SanitizeHtmlTest extends PHPUnit_Framework_TestCase
{
public function testServiceCanBeRetrieved()
{
$serviceManager = Bootstrap::getServiceManager();

/* @var \Zend\View\HelperPluginManager $viewHelperManager */
$viewHelperManager = $serviceManager->get('ViewHelperManager');

$this->assertInstanceOf(
SanitizeHtml::class,
$viewHelperManager->get('sanitizeHtml')
);
}

public function testHtmlGetsCleaned()
{
$serviceManager = Bootstrap::getServiceManager();

/* @var \Zend\View\HelperPluginManager $viewHelperManager */
$viewHelperManager = $serviceManager->get('ViewHelperManager');

/* @var \Application\View\Helper\SanitizeHtml $sanitizeHtmlHelper */
$sanitizeHtmlHelper = $viewHelperManager->get('sanitizeHtml');

$dirtyHtml = 'Foo<script>alert(\'I_WILL_BE_REMOVED\');</script>Bar';
$this->assertEquals('FooBar', $sanitizeHtmlHelper->__invoke($dirtyHtml));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace ApplicationTest\Integration\View\Helper;

use Application\Service\HtmlPurifierFactory;
use PHPUnit_Framework_TestCase;
use Zend\ServiceManager\ServiceManager;

class HtmlPurifierTest extends PHPUnit_Framework_TestCase
{
public function testConfigCanBePassedToService()
{
$config = [
'htmlpurifier' => [
'HTML.AllowedElements' => 'foo'
]
];

$serviceMock = $this->getMock(ServiceManager::class, ['get']);
$serviceMock
->expects($this->once())
->method('get')
->with($this->equalTo('Config'))
->willReturn($config);

$factory = new HtmlPurifierFactory();
$htmlPurifierInstance = $factory->createService($serviceMock);

$this->assertArrayHasKey('foo', $htmlPurifierInstance->config->get('HTML.AllowedElements'));
}
}
103 changes: 51 additions & 52 deletions module/Application/view/application/index/index.phtml
Original file line number Diff line number Diff line change
Expand Up @@ -18,66 +18,65 @@
</div>
</div>
</div>
<?php foreach($this->repositories as $module) { ?>
<div class="row-fluid module-row">
<div class="span12">
<div class="row-fluid">
<div class="module-info">
<div class="span6 module-author">
<div class="row-fluid">
<div class="span3">
<img src="<?php echo $module->getPhotoUrl()?>" alt="<?php echo $module->getOwner()?>" class="avatar">
</div>
<div class="span9">
<strong><?php echo $module->getOwner()?></strong>
<p>
<span class="author-label">Github:</span>
<a href="<?php echo $module->getUrl()?>"><?php echo $module->getName()?></a>
<!--
<span class="author-label">Blog:</span> <a>placeholder</a><br />
<span class="author-label">Modules:</span> <a>5</a><br /><br />
<span class="author-label"><a>More info...</a>-->
</p>
<?php foreach($this->repositories as $module): ?>
<div class="row-fluid module-row">
<div class="span12">
<div class="row-fluid">
<div class="module-info">
<div class="span6 module-author">
<div class="row-fluid">
<div class="span3">
<img src="<?php echo $this->escapeHtmlAttr($module->getPhotoUrl()); ?>" alt="<?php echo $this->escapeHtmlAttr($module->getOwner()); ?>" class="avatar">
</div>
<div class="span9">
<strong><?php echo $this->escapeHtml($module->getOwner()); ?></strong>
<p>
<span class="author-label">Github:</span>
<a href="<?php echo $this->escapeHtmlAttr($module->getUrl()); ?>"><?php echo $this->escapeHtml($module->getName()); ?></a>
<!--
<span class="author-label">Blog:</span> <a>placeholder</a><br />
<span class="author-label">Modules:</span> <a>5</a><br /><br />
<span class="author-label"><a>More info...</a>-->
</p>
</div>
</div>
</div>
<div class="span6">
<strong>
<a href="<?php echo $this->url('view-module', array('vendor' => $this->escapeUrl($module->getOwner()),'module' => $this->escapeUrl($module->getName())));?>">
<?php echo $this->escapeHtml($module->getName()); ?>
</a>
</strong>
<p>
<!--<span class="author-label">Version:</span> 1.2.1<br />-->
<span class="author-label">Created:</span> <?php
$date = new DateTime($module->getCreatedAt());
echo $date->format('Y-m-d');
?><br>
<!--<span class="author-label">Edited:</span> <a>15 days ago</a><br />
<span class="author-label">Tags:</span>
<span class="label">zf2</span>
<span class="label">Assetic</span>
<span class="label">Assets</span>
<span class="label">Sx</span>-->
</p>
</div>
<div style="clear: both;"></div>
</div>
<div class="span6">
<strong><a href="<?php
echo
$this->url('view-module', array(
'vendor' => $module->getOwner(),
'module' => $module->getName(),
)
);?>"><?php echo $module->getName()?></a></strong>
<p>
<!--<span class="author-label">Version:</span> 1.2.1<br />-->
<span class="author-label">Created:</span> <?php
$date = new DateTime($module->getCreatedAt());
echo $date->format('Y-m-d');
?><br>
<!--<span class="author-label">Edited:</span> <a>15 days ago</a><br />
<span class="author-label">Tags:</span>
<span class="label">zf2</span>
<span class="label">Assetic</span>
<span class="label">Assets</span>
<span class="label">Sx</span>-->
</p>
</div>
<div style="clear: both;"></div>
</div>
</div>
<div class="row-fluid">
<div class="module-description">
<div class="span12">
<p>
<?php echo $module->getDescription()?>
</p>
<div class="row-fluid">
<div class="module-description">
<div class="span12">
<p>
<?php echo $this->escapeHtml($module->getDescription()); ?>
</p>
</div>
<div style="clear: both;"></div>
</div>
<div style="clear: both;"></div>
</div>
</div>
</div>
</div><?php } ?>
<?php endforeach; ?>
<?php echo $this->paginationControl($this->repositories, 'Sliding', 'application/index/pagination', array('query' =>$query)); ?>
</div>
<div class="span4">
Expand Down
10 changes: 5 additions & 5 deletions module/Application/view/application/search/index.phtml
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
<?php foreach($results as $module) {?>
<?php foreach($results as $module): ?>
<div class="row-fluid">
<div class="span12">
<div class="span2">
<img src="<?php echo $module->getPhotoUrl()?>" alt="<?php echo $module->getName()?>" class="avatar">
<img src="<?php echo $this->escapeUrl($module->getPhotoUrl()); ?>" alt="<?php echo $this->escapeHtmlAttr($module->getName()); ?>" class="avatar">
</div>
<div class="span10">
<a href="<?php echo $module->getUrl()?>"><?php echo $module->getName()?></a><br>
<?php echo $module->getDescription()?>
<a href="<?php echo $this->escapeUrl($module->getUrl()); ?>"><?php echo $this->escapeHtml($module->getName()); ?></a><br>
<?php echo $this->escapeHtml($module->getDescription()); ?>
</div>
</div>
</div>
<hr style="margin:2px;padding:0;"/>
<?php } ?>
<?php endforeach; ?>
6 changes: 3 additions & 3 deletions module/Application/view/layout/layout-small-header.phtml
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@
echo $this->url('zfcuser');
?>" title="My profile">
<img src="<?php
echo $this->zfcUserIdentity()->getPhotoUrl();
?>" alt="<?php echo $this->zfcUserDisplayName();
echo $this->escapeHtmlAttr($this->zfcUserIdentity()->getPhotoUrl());
?>" alt="<?php echo $this->escapeHtmlAttr($this->zfcUserDisplayName());
?>" style="width:23px;height:23px;" />
</a></div><a class="login" href="<?php
echo $this->url('zfcuser');
?>" title="My profile"> Hello <?php echo ucfirst($userDisplayName);?></a>
?>" title="My profile"> Hello <?php echo $this->escapeHtml(ucfirst($userDisplayName));?></a>
</li>
<li class="sep">|</li>
<li ><a href="<?php echo $this->url('zfcuser/logout'); ?>">Logout</a></li>
Expand Down
9 changes: 4 additions & 5 deletions module/Application/view/layout/layout.phtml
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,15 @@
?>
<li class="sep">|</li>
<li class="user-info">
<div class="gravatar-container"><a class="login" href="<?php
echo $this->url('zfcuser');
<div class="gravatar-container"><a class="login" href="<?php echo $this->url('zfcuser');
?>" title="My profile">
<img src="<?php
echo $this->zfcUserIdentity()->getPhotoUrl();
?>" alt="<?php echo $this->zfcUserDisplayName();
echo $this->escapeHtmlAttr($this->zfcUserIdentity()->getPhotoUrl());
?>" alt="<?php echo $this->escapeHtmlAttr($this->zfcUserDisplayName());
?>" style="width:23px;height:23px;" />
</a></div><a class="login" href="<?php
echo $this->url('zfcuser');
?>" title="My profile"> Hello <?php echo ucfirst($userDisplayName);?></a>
?>" title="My profile"> Hello <?php echo $this->escapeHtml(ucfirst($userDisplayName));?></a>
</li>
<li class="sep">|</li>
<li ><a href="<?php echo $this->url('zfcuser/logout'); ?>">Logout</a></li>
Expand Down
4 changes: 2 additions & 2 deletions module/User/view/user/helper/new-users.phtml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
<div class="span12">
<?php } ?>
<div class="span3">
<a href="https://github.com/<?php echo $user->getUserName()?>" data-delay="0" rel="tooltip" title="<?php echo $user->getUserName()?>" class="thumbnail">
<img src="<?php echo $user->getPhotoUrl()?>" alt="<?php echo $user->getUserName()?>">
<a href="https://github.com/<?php echo $this->escapeHtml($user->getUserName()); ?>" data-delay="0" rel="tooltip" title="<?php echo $this->escapeHtml($user->getUserName()); ?>" class="thumbnail">
<img src="<?php echo $user->getPhotoUrl()?>" alt="<?php echo $this->escapeHtml($user->getUserName()); ?>">
</a>
</div>
<?php if($count%4 ==3){ ?>
Expand Down
Loading

0 comments on commit 4ad4a78

Please sign in to comment.