From 0d55bc7f420470e0dbca91ebe7899c592905cbc5 Mon Sep 17 00:00:00 2001 From: Johan Cwiklinski Date: Tue, 9 Nov 2021 08:35:51 +0100 Subject: [PATCH] Fix XSS, prevent their storage Use HTML purifier lib to sanitize preferences footer --- galette/composer.json | 3 +- galette/composer.lock | 56 ++++++++++++++++++- .../Crud/PaymentTypeController.php | 14 ++--- galette/lib/Galette/Core/Preferences.php | 20 +++++++ galette/lib/Galette/Entity/Entitled.php | 2 + galette/lib/Galette/Entity/Title.php | 4 +- galette/lib/Galette/Entity/Transaction.php | 2 +- .../default/gestion_intitule_content.tpl | 6 +- galette/templates/default/gestion_titres.tpl | 10 ++-- .../default/gestion_transactions.tpl | 2 +- 10 files changed, 98 insertions(+), 21 deletions(-) diff --git a/galette/composer.json b/galette/composer.json index 7188763d5a..f33d8d0058 100644 --- a/galette/composer.json +++ b/galette/composer.json @@ -49,7 +49,8 @@ "php-di/slim-bridge": "2.0.0", "doctrine/annotations": "^1.8", "laminas/laminas-servicemanager": "3.7", - "symfony/polyfill-php80": "^1.23" + "symfony/polyfill-php80": "^1.23", + "ezyang/htmlpurifier": "^4.13" }, "require-dev": { "atoum/atoum": "dev-master", diff --git a/galette/composer.lock b/galette/composer.lock index 41ae74895e..79e9819a4e 100644 --- a/galette/composer.lock +++ b/galette/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "abdb30cee5eeb0dad05c159a1c10880d", + "content-hash": "47d56d47701d9038105bd93cf3b44a02", "packages": [ { "name": "akrabat/rka-slim-session-middleware", @@ -366,6 +366,60 @@ ], "time": "2020-05-25T17:44:05+00:00" }, + { + "name": "ezyang/htmlpurifier", + "version": "v4.13.0", + "source": { + "type": "git", + "url": "https://github.com/ezyang/htmlpurifier.git", + "reference": "08e27c97e4c6ed02f37c5b2b20488046c8d90d75" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/ezyang/htmlpurifier/zipball/08e27c97e4c6ed02f37c5b2b20488046c8d90d75", + "reference": "08e27c97e4c6ed02f37c5b2b20488046c8d90d75", + "shasum": "" + }, + "require": { + "php": ">=5.2" + }, + "require-dev": { + "simpletest/simpletest": "dev-master#72de02a7b80c6bb8864ef9bf66d41d2f58f826bd" + }, + "type": "library", + "autoload": { + "psr-0": { + "HTMLPurifier": "library/" + }, + "files": [ + "library/HTMLPurifier.composer.php" + ], + "exclude-from-classmap": [ + "/library/HTMLPurifier/Language/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "LGPL-2.1-or-later" + ], + "authors": [ + { + "name": "Edward Z. Yang", + "email": "admin@htmlpurifier.org", + "homepage": "http://ezyang.com" + } + ], + "description": "Standards compliant HTML filter written in PHP", + "homepage": "http://htmlpurifier.org/", + "keywords": [ + "html" + ], + "support": { + "issues": "https://github.com/ezyang/htmlpurifier/issues", + "source": "https://github.com/ezyang/htmlpurifier/tree/master" + }, + "time": "2020-06-29T00:56:53+00:00" + }, { "name": "laminas/laminas-cache", "version": "2.13.0", diff --git a/galette/lib/Galette/Controllers/Crud/PaymentTypeController.php b/galette/lib/Galette/Controllers/Crud/PaymentTypeController.php index f5072d48a5..af1796e71b 100644 --- a/galette/lib/Galette/Controllers/Crud/PaymentTypeController.php +++ b/galette/lib/Galette/Controllers/Crud/PaymentTypeController.php @@ -7,7 +7,7 @@ * * PHP version 5 * - * Copyright © 2019-2020 The Galette Team + * Copyright © 2019-2021 The Galette Team * * This file is part of Galette (http://galette.tuxfamily.org). * @@ -28,7 +28,7 @@ * @package Galette * * @author Johan Cwiklinski - * @copyright 2019-2020 The Galette Team + * @copyright 2019-2021 The Galette Team * @license http://www.gnu.org/licenses/gpl-3.0.html GPL License 3.0 or (at your option) any later version * @link http://galette.tuxfamily.org * @since Available since 0.9.4dev - 2019-12-09 @@ -50,7 +50,7 @@ * @name PaymentTypeController * @package Galette * @author Johan Cwiklinski - * @copyright 2019-2020 The Galette Team + * @copyright 2019-2021 The Galette Team * @license http://www.gnu.org/licenses/gpl-3.0.html GPL License 3.0 or (at your option) any later version * @link http://galette.tuxfamily.org * @since Available since 0.9.4dev - 2019-12-09 @@ -205,7 +205,7 @@ public function store(Request $request, Response $response, int $id = null): Res 'error_detected', preg_replace( '(%s)', - $ptype->name, + $ptype->getName(), _T("Payment type '%s' has not been added!") ) ); @@ -214,7 +214,7 @@ public function store(Request $request, Response $response, int $id = null): Res 'error_detected', preg_replace( '(%s)', - $ptype->name, + $ptype->getName(), _T("Payment type '%s' has not been modified!") ) ); @@ -227,7 +227,7 @@ public function store(Request $request, Response $response, int $id = null): Res 'success_detected', preg_replace( '(%s)', - $ptype->name, + $ptype->getName(), _T("Payment type '%s' has been successfully added.") ) ); @@ -236,7 +236,7 @@ public function store(Request $request, Response $response, int $id = null): Res 'success_detected', preg_replace( '(%s)', - $ptype->name, + $ptype->getName(), _T("Payment type '%s' has been successfully modified.") ) ); diff --git a/galette/lib/Galette/Core/Preferences.php b/galette/lib/Galette/Core/Preferences.php index 66395fa14d..e488c4c8ce 100644 --- a/galette/lib/Galette/Core/Preferences.php +++ b/galette/lib/Galette/Core/Preferences.php @@ -755,6 +755,9 @@ public function validateValue($fieldname, $value) $this->errors[] = _T("- Invalid year for cards."); } break; + case 'pref_footer': + $value = $this->cleanHtmlValue($value); + break; } return $value; @@ -961,6 +964,8 @@ public function __get($name) && $name == 'pref_mail_method' ) { return GaletteMail::METHOD_DISABLED; + } elseif ($name == 'pref_footer') { + return $this->cleanHtmlValue($this->prefs[$name]); } else { if ($name == 'pref_adhesion_form' && $this->prefs[$name] == '') { $this->prefs[$name] = self::$defaults['pref_adhesion_form']; @@ -1294,4 +1299,19 @@ public function setSocialReplacements(): self return $this; } + + /** + * Purify HTML value + * + * @param string $value Value to clean + * + * @return string + */ + public function cleanHtmlValue(string $value): string + { + $config = \HTMLPurifier_Config::createDefault(); + $config->set('Cache.SerializerPath', GALETTE_CACHE_DIR); + $purifier = new \HTMLPurifier($config); + return $purifier->purify($value); + } } diff --git a/galette/lib/Galette/Entity/Entitled.php b/galette/lib/Galette/Entity/Entitled.php index 20e71edbb5..b4671a28dc 100644 --- a/galette/lib/Galette/Entity/Entitled.php +++ b/galette/lib/Galette/Entity/Entitled.php @@ -429,6 +429,7 @@ public function getIdByLabel($label) public function add($label, $extra) { // Avoid duplicates. + $label = strip_tags($label); $ret = $this->getIdByLabel($label); if ($ret !== false) { @@ -489,6 +490,7 @@ public function add($label, $extra) */ public function update($id, $label, $extra) { + $label = strip_tags($label); $ret = $this->get($id); if (!$ret) { /* get() already logged and set $this->error. */ diff --git a/galette/lib/Galette/Entity/Title.php b/galette/lib/Galette/Entity/Title.php index 3218522f09..dfdc2d174a 100644 --- a/galette/lib/Galette/Entity/Title.php +++ b/galette/lib/Galette/Entity/Title.php @@ -139,8 +139,8 @@ private function loadFromRs($rs) public function store($zdb) { $data = array( - 'short_label' => $this->short, - 'long_label' => $this->long + 'short_label' => strip_tags($this->short), + 'long_label' => strip_tags($this->long) ); try { if ($this->id !== null && $this->id > 0) { diff --git a/galette/lib/Galette/Entity/Transaction.php b/galette/lib/Galette/Entity/Transaction.php index 08caee36c5..799d5e91af 100644 --- a/galette/lib/Galette/Entity/Transaction.php +++ b/galette/lib/Galette/Entity/Transaction.php @@ -357,7 +357,7 @@ public function check($values, $required, $disabled) break; case 'trans_desc': /** TODO: retrieve field length from database and check that */ - $this->_description = $value; + $this->_description = strip_tags($value); if (mb_strlen($value) > 150) { $this->errors[] = _T("- Transaction description must be 150 characters long maximum."); } diff --git a/galette/templates/default/gestion_intitule_content.tpl b/galette/templates/default/gestion_intitule_content.tpl index ee9f05a099..a6cfa2a85e 100644 --- a/galette/templates/default/gestion_intitule_content.tpl +++ b/galette/templates/default/gestion_intitule_content.tpl @@ -57,7 +57,7 @@ {$eid} $url_class, "action" => "edit", "id" => $eid]}"> - {_T string="%s field" pattern="/%s/" replace=$entry.name} + {_T string="%s field" pattern="/%s/" replace=$entry.name|escape} @@ -89,14 +89,14 @@ class="action tooltip" > - {_T string="Edit '%s' field" pattern="/%s/" replace=$entry.name} + {_T string="Edit '%s' field" pattern="/%s/" replace=$entry.name|escape} $url_class, "id" => $eid]}" class="delete tooltip" > - {_T string="Delete '%s' field" pattern="/%s/" replace=$entry.name} + {_T string="Delete '%s' field" pattern="/%s/" replace=$entry.name|escape} diff --git a/galette/templates/default/gestion_titres.tpl b/galette/templates/default/gestion_titres.tpl index 52ed939c85..762e32dea2 100644 --- a/galette/templates/default/gestion_titres.tpl +++ b/galette/templates/default/gestion_titres.tpl @@ -44,19 +44,19 @@ {/if} $title->id]}"> - {_T string="%s title" pattern="/%s/" replace=$title->short} + {_T string="%s title" pattern="/%s/" replace=$title->short|escape} - {$title->short} - {$title->long} + {$title->short|escape} + {$title->long|escape} $title->id]}" class="tooltip action" > - {_T string="Edit '%s' title" pattern="/%s/" replace=$title->short} + {_T string="Edit '%s' title" pattern="/%s/" replace=$title->short|escape} {if $title->id eq 1 or $title->id eq 2} @@ -66,7 +66,7 @@ class="delete tooltip" > - {_T string="Delete '%s' title" pattern="/%s/" replace=$title->short} + {_T string="Delete '%s' title" pattern="/%s/" replace=$title->short|escape} {/if} diff --git a/galette/templates/default/gestion_transactions.tpl b/galette/templates/default/gestion_transactions.tpl index b812baf43b..69273777eb 100644 --- a/galette/templates/default/gestion_transactions.tpl +++ b/galette/templates/default/gestion_transactions.tpl @@ -150,7 +150,7 @@ {/if} - {$transaction->description} + {$transaction->description|escape} {if $login->isAdmin() or $login->isStaff()} {if $filters->filtre_cotis_adh eq ""}