From 48190f48129d80a69fb0e7fbbb1c8e96766d30e6 Mon Sep 17 00:00:00 2001 From: Patrick Barroca <pbarroca@afi-sa.fr> Date: Wed, 25 Sep 2019 17:05:09 +0200 Subject: [PATCH] hotline #97784 : fix XSS on many js attributes --- VERSIONS_HOTLINE/97784 | 1 + .../View/Helper/Abonne/ReservationsTable.php | 10 +- .../ZendAfi/View/Helper/Admin/FileManager.php | 9 +- library/ZendAfi/View/Helper/BaseHelper.php | 5 + library/ZendAfi/View/Helper/Button.php | 8 +- .../DigitalResource/Dashboard/Harvest.php | 3 +- .../ZendAfi/View/Helper/ReseauxSociaux.php | 3 +- library/ZendAfi/View/Helper/SelectWidget.php | 3 +- tests/scenarios/Security/CmsTest.php | 43 ++++++ tests/scenarios/Security/FileManagerTest.php | 132 ++++++++++++++++++ tests/scenarios/Security/SelectWidgetTest.php | 50 +++++++ 11 files changed, 250 insertions(+), 17 deletions(-) create mode 100644 VERSIONS_HOTLINE/97784 create mode 100644 tests/scenarios/Security/CmsTest.php create mode 100644 tests/scenarios/Security/FileManagerTest.php create mode 100644 tests/scenarios/Security/SelectWidgetTest.php diff --git a/VERSIONS_HOTLINE/97784 b/VERSIONS_HOTLINE/97784 new file mode 100644 index 00000000000..4886856f704 --- /dev/null +++ b/VERSIONS_HOTLINE/97784 @@ -0,0 +1 @@ + - ticket #97784 : Sécurité : Correction de failles XSS \ No newline at end of file diff --git a/library/ZendAfi/View/Helper/Abonne/ReservationsTable.php b/library/ZendAfi/View/Helper/Abonne/ReservationsTable.php index 71e65ecd0ff..a7ec5b90914 100644 --- a/library/ZendAfi/View/Helper/Abonne/ReservationsTable.php +++ b/library/ZendAfi/View/Helper/Abonne/ReservationsTable.php @@ -173,10 +173,12 @@ class ZendAfi_View_Helper_Abonne_OnPlaceConsultationReservationsTable extends Ze protected function _getCancelLink($reservation_id) { + $img = $this->view->tagImg(URL_IMG . 'bouton/cancel.gif', + ['onclick' => 'return confirm(\'' + . $this->_escapeJsAttrib($this->_('Etes vous sûr de vouloir supprimer cette réservation ?')) . '\')', + 'title' => $this->_('Supprimer cette réservation')]); + return $this->view->tagAnchor($this->view->url(['consultation_id_delete' => $reservation_id]), - $this->view->tagImg(URL_IMG . 'bouton/cancel.gif', - ['onclick' => 'return confirm(\''. str_replace("'", "\\'", $this->_('Etes vous sûr de vouloir supprimer cette réservation ?')) . '\')', - 'title' => $this->_('Supprimer cette réservation')])); + $img); } } -?> \ No newline at end of file diff --git a/library/ZendAfi/View/Helper/Admin/FileManager.php b/library/ZendAfi/View/Helper/Admin/FileManager.php index 96bbab4ef64..0345b08187d 100644 --- a/library/ZendAfi/View/Helper/Admin/FileManager.php +++ b/library/ZendAfi/View/Helper/Admin/FileManager.php @@ -336,9 +336,9 @@ class ZendAfi_View_Helper_Admin_FileManager extends ZendAfi_View_Helper_BaseHelp $url = $this->view->url(['focused_browser' => $waiting_browser]); if($this->view->isPopup()) - return sprintf('opacDialogFromUrl(\'%s\');', $url); + return sprintf('opacDialogFromUrl(\'%s\');', $this->_escapeJsAttrib($url)); - return sprintf('location.href = \'%s\';return true;', $url); + return sprintf('location.href = \'%s\';return true;', $this->_escapeJsAttrib($url)); } @@ -443,8 +443,8 @@ class ZendAfi_View_Helper_Admin_FileManager extends ZendAfi_View_Helper_BaseHelp $url = $this->view->url(['search_' . $key => null]) . '&search_' . $key; $onclick = $this->view->isPopup() - ? sprintf("var value = encodeURIComponent($(this).closest('div').find('input').val());opacDialogFromUrl(addPath('%s=' + value, '&render=popup'));", $url) - : sprintf("var value = encodeURIComponent($(this).closest('div').find('input').val());document.location = '%s=' + value;", $url); + ? sprintf("var value = encodeURIComponent($(this).closest('div').find('input').val());opacDialogFromUrl(addPath('%s=' + value, '&render=popup'));", $this->_escapeJsAttrib($url)) + : sprintf("var value = encodeURIComponent($(this).closest('div').find('input').val());document.location = '%s=' + value;", $this->_escapeJsAttrib($url)); return $this->_tag('input', null, @@ -453,6 +453,7 @@ class ZendAfi_View_Helper_Admin_FileManager extends ZendAfi_View_Helper_BaseHelp 'placeholder' => $term ? $term : $this->_('Rechercher dans "%s"', $item->getPath()), 'name' => 'search_' . $key, 'onkeypress' => sprintf('if (event.keyCode==13) {%s}', $onclick)]) . + $this->view->button((new Class_Entity) ->setText($this->_tag('i', '', ['class' => 'fa fa-search'])) ->setTitle($this->_('Rechercher le terme saisie')) diff --git a/library/ZendAfi/View/Helper/BaseHelper.php b/library/ZendAfi/View/Helper/BaseHelper.php index 1742041c9f6..a2b9340f6e7 100644 --- a/library/ZendAfi/View/Helper/BaseHelper.php +++ b/library/ZendAfi/View/Helper/BaseHelper.php @@ -81,4 +81,9 @@ class ZendAfi_View_Helper_BaseHelper extends Zend_View_Helper_HtmlElement { protected function _url(array $urlOptions = [], $name = null, $reset = false, $encode = true) { return call_user_func_array([$this->view, 'url'], func_get_args()); } + + + protected function _escapeJsAttrib($value) { + return str_replace(['\'', '"'], '\\\'', $value); + } } \ No newline at end of file diff --git a/library/ZendAfi/View/Helper/Button.php b/library/ZendAfi/View/Helper/Button.php index aa1c22ad12d..0b0f4f60125 100644 --- a/library/ZendAfi/View/Helper/Button.php +++ b/library/ZendAfi/View/Helper/Button.php @@ -61,19 +61,19 @@ class ZendAfi_View_Helper_Button extends ZendAfi_View_Helper_BaseHelper { $button->setAttribs(array_merge($button->getAttribs(), ['onclick' => sprintf('window.location.href = \'%s\';', - $button->getUrl())])); + $this->_escapeJsAttrib($button->getUrl()))])); return $this; } protected function _defaultsWithConfirm($button) { $action = $this->view->isPopup() - ? "opacDialogClose(); opacDialogFromUrl('" . $button->getUrl() ."');" - : "window.location.href='" . $button->getUrl() ."'"; + ? "opacDialogClose(); opacDialogFromUrl('" . $this->_escapeJsAttrib($button->getUrl()) ."');" + : "window.location.href='" . $this->_escapeJsAttrib($button->getUrl()) ."'"; $button->setAttribs(array_merge($button->getAttribs(), ['onclick' => sprintf("if (confirm('%s')) { %s }; return false;", - str_replace(['\'', '"'], '\\\'', $button->getConfirm()), + $this->_escapeJsAttrib($button->getConfirm()), $action)])); return $this; } diff --git a/library/ZendAfi/View/Helper/DigitalResource/Dashboard/Harvest.php b/library/ZendAfi/View/Helper/DigitalResource/Dashboard/Harvest.php index 2664dc2f37e..475566b551d 100644 --- a/library/ZendAfi/View/Helper/DigitalResource/Dashboard/Harvest.php +++ b/library/ZendAfi/View/Helper/DigitalResource/Dashboard/Harvest.php @@ -95,8 +95,7 @@ class ZendAfi_View_Helper_DigitalResource_Dashboard_Harvest extends ZendAfi_View }, 'anchorOptions' => ['onclick' => 'return confirm(\'' - . str_replace(['\'', '"'], '\\\'', - $this->_('Etes-vous sur de vouloir désactiver cette tâche ?')) + . $this->_escapeJsAttrib($this->_('Etes-vous sur de vouloir désactiver cette tâche ?')) . '\')']], ['url' => '/admin/batch/plan/id/%s', diff --git a/library/ZendAfi/View/Helper/ReseauxSociaux.php b/library/ZendAfi/View/Helper/ReseauxSociaux.php index f826ca84ee5..4938a514584 100644 --- a/library/ZendAfi/View/Helper/ReseauxSociaux.php +++ b/library/ZendAfi/View/Helper/ReseauxSociaux.php @@ -113,7 +113,8 @@ class ZendAfi_View_Helper_ReseauxSociaux extends ZendAfi_View_Helper_BaseHelper ['class' => 'reseau-social-img', 'alt' => $label, 'title' => $label, - 'onclick' => sprintf('$.getScript(\'%s\')', $this->getScriptFromController($url_table))]); + 'onclick' => sprintf('$.getScript(\'%s\')', + $this->_escapeJsAttrib($this->getScriptFromController($url_table)))]); } diff --git a/library/ZendAfi/View/Helper/SelectWidget.php b/library/ZendAfi/View/Helper/SelectWidget.php index eca16250595..6c80aebdf6b 100644 --- a/library/ZendAfi/View/Helper/SelectWidget.php +++ b/library/ZendAfi/View/Helper/SelectWidget.php @@ -30,7 +30,7 @@ class ZendAfi_View_Helper_SelectWidget extends ZendAfi_View_Helper_BaseHelper { 'action' => 'simple', $key => null]); - $onchange = "var format=$('#" . $key . "').val();document.location='" . $url_str . "/" . $key . "/'+format;"; + $onchange = "var format=$('#" . $key . "').val();document.location='" . $this->_escapeJsAttrib($url_str . "/" . $key) . "/'+format;"; $html = $this->_tag('label', $settings->getLabel(), ['for' => $key]) . ' ' . @@ -48,4 +48,3 @@ class ZendAfi_View_Helper_SelectWidget extends ZendAfi_View_Helper_BaseHelper { 'data-key' => $key]); } } -?> \ No newline at end of file diff --git a/tests/scenarios/Security/CmsTest.php b/tests/scenarios/Security/CmsTest.php new file mode 100644 index 00000000000..7cc7e43164e --- /dev/null +++ b/tests/scenarios/Security/CmsTest.php @@ -0,0 +1,43 @@ +<?php +/** + * Copyright (c) 2012-2019, Agence Française Informatique (AFI). All rights reserved. + * + * BOKEH is free software; you can redistribute it and/or modify + * it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE as published by + * the Free Software Foundation. + * + * There are special exceptions to the terms and conditions of the AGPL as it + * is applied to this software (see README file). + * + * BOKEH is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU AFFERO GENERAL PUBLIC LICENSE + * along with BOKEH; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + + +class Security_CmsTest extends AbstractControllerTestCase { + protected $_storm_default_to_volatile = true; + + + public function setUp() { + parent::setUp(); + + $this->fixture('Class_Article', + ['id' => 464, + 'titre' => 'Un titre', + 'contenu' => 'Un contenu']); + + $this->dispatch('/opac/cms/articleview/id_module/9/ids/464/strategy/Article_List/modele_fusion/8/id/464/"><svg onload=alert(1)>'); + } + + + /** @test */ + public function pageShouldNotContainsInjectedSVG() { + $this->assertNotXPath('//svg[@onload]', $this->_response->getBody()); + } +} diff --git a/tests/scenarios/Security/FileManagerTest.php b/tests/scenarios/Security/FileManagerTest.php new file mode 100644 index 00000000000..b7417782f4e --- /dev/null +++ b/tests/scenarios/Security/FileManagerTest.php @@ -0,0 +1,132 @@ +<?php +/** + * Copyright (c) 2012-2019, Agence Française Informatique (AFI). All rights reserved. + * + * BOKEH is free software; you can redistribute it and/or modify + * it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE as published by + * the Free Software Foundation. + * + * There are special exceptions to the terms and conditions of the AGPL as it + * is applied to this software (see README file). + * + * BOKEH is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU AFFERO GENERAL PUBLIC LICENSE + * along with BOKEH; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + + +class Security_FileManagerTest extends ViewHelperTestCase { + protected + $_storm_default_to_volatile = true, + $_html; + + + public function setUp() { + parent::setUp(); + + $this->_userfiles_dir = (new Class_FileManager) + ->setId('userfiles') + ->setDir(true) + ->setName('userfiles') + ->setPath('userfiles'); + + $skins_dir = (new Class_FileManager) + ->setId('skins') + ->setDir(true) + ->setName('skins') + ->setPath('skins'); + + $this->_dir = (new Class_FileManager) + ->setId('userfiles/image') + ->setParentPath('userfiles') + ->setPath('userfiles/image') + ->setName('image') + ->setDir(true); + + $disk = $this->mock() + ->whenCalled('diskSpaceInfo') + ->answers((new Class_Entity) + ->setId('userfiles') + ->setFree('2 GO') + ->setUsed('153 GO') + ->setTotal('155 GO') + ->setPercent('99%') + ->whenCalledDo('isFull', function(){return false;})) + + ->whenCalled('clearCache') + ->answers(true) + + ->whenCalled('isOversized') + ->answers(false) + + ->whenCalled('directoriesAt') + ->with('userfiles') + ->answers([$this->_dir]) + + ->whenCalled('directoriesAt') + ->with('userfiles/image') + ->answers([]) + + ->whenCalled('directoryAt') + ->with('userfiles/image') + ->answers($skins_dir) + + ->whenCalled('filesAt') + ->with('userfiles/image') + ->answers([]) + + ->whenCalled('directoryAt') + ->with('userfiles') + ->answers($this->_userfiles_dir) + + ->whenCalled('filesAt') + ->with('userfiles') + ->answers([]) + + ->whenCalled('directoryAt') + ->with('skins') + ->answers($skins_dir) + + ->whenCalled('directoriesAt') + ->with('skins') + ->answers([]) + + ->whenCalled('filesAt') + ->with('skins') + ->answers([]) + ; + + Class_FileManager::setFileSystem($disk); + + $this->login(ZendAfi_Acl_AdminControllerroles::SUPER_ADMIN); + + Zend_Controller_Front::getInstance() + ->getRouter() + ->route(new Zend_Controller_Request_Http('http://localhost/admin/filemanager/index/' + . urlencode('"><svg onload=alert(1)>'))); + + $helper = new ZendAfi_View_Helper_Admin_FileManager(); + $helper->setView($this->view); + + $this->_html = $helper->fileManager((new Class_Entity) + ->setBrowser('userfiles') + ->setUser(Class_Users::getIdentity())); + } + + + public function tearDown() { + Class_FileManager::setFileSystem(null); + parent::tearDown(); + } + + + /** @test */ + public function pageShouldNotContainsInjectedSVG() { + $this->assertNotXPath($this->_html, '//svg[@onload]', $this->_html); + } +} diff --git a/tests/scenarios/Security/SelectWidgetTest.php b/tests/scenarios/Security/SelectWidgetTest.php new file mode 100644 index 00000000000..97bc7c0566f --- /dev/null +++ b/tests/scenarios/Security/SelectWidgetTest.php @@ -0,0 +1,50 @@ +<?php +/** + * Copyright (c) 2012-2019, Agence Française Informatique (AFI). All rights reserved. + * + * BOKEH is free software; you can redistribute it and/or modify + * it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE as published by + * the Free Software Foundation. + * + * There are special exceptions to the terms and conditions of the AGPL as it + * is applied to this software (see README file). + * + * BOKEH is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU AFFERO GENERAL PUBLIC LICENSE + * along with BOKEH; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + + +class Security_SelectWidgetTest extends ViewHelperTestCase { + protected + $_storm_default_to_volatile = true, + $_html; + + + public function setUp() { + parent::setUp(); + + $this->login(ZendAfi_Acl_AdminControllerroles::SUPER_ADMIN); + + Zend_Controller_Front::getInstance() + ->getRouter() + ->route(new Zend_Controller_Request_Http('http://localhost/admin/filemanager/index/' + . urlencode('"><svg onload=alert(1)>'))); + + $helper = new ZendAfi_View_Helper_SelectWidget(); + $helper->setView($this->view); + + $this->_html = $helper->selectWidget((new Class_Entity)); + } + + + /** @test */ + public function pageShouldNotContainsInjectedSVG() { + $this->assertNotXPath($this->_html, '//svg[@onload]', $this->_html); + } +} -- GitLab