From 989a4cec547fe6eab25d3e0816d47ba9ceff2f17 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 20 Mar 2024 10:57:16 -0700 Subject: [PATCH 1/4] Html Writer Comments - Breaking Change Fix #3954. In response to a Security Incident, a package was added to the project to sanitize the Html for comments attached to a cell. We have tried at least 2 different packages for this purpose, and users have raised legitimate concerns about both. I believe that adding a sanitizer package, although it addresses the problem, was overkill. Cell comments are a RichText object, and Html Writer already handled RichText cell *values*. Values did not figure in the Incident, and they surely would have done so if they were a problem because it is a lot easier to set up test cases for values than for comments. RichText values were not a problem because they were sanitized with `htmlspecialchars`; if comments were to use the same code that RichText was already using, it would likewise be safely sanitized. As an added bonus, the existing code for comments only uses the plaintext value, but the values code would allow the comments to be styled, just as they are for Xlsx. This is a breaking change - I don't think it will affect a lot of users, but there may be some. It is worth noting that the comment block before function `writeComment` has a link explaining what is being done. That link mentions only styling elements, not other possibilities like hyperlinks. At any rate, if people are putting styling tags (e.g. ``) in their comments for this purpose, those will no longer work; the styling needs to be applied to the RichText elements. That will keep the user code the same regardless of the format of the intended output, which is certainly a good thing, but it is a break. If people are trying to insert non-styling tage (e.g. ``), those will no longer work, just as there is no way to do it for Xlsx (although Excel itself may convert the raw text to a hyperlink, but that's out of scope, at least for now). I also note that, even with the current code, I haven't yet found a way to keep the Html comment in place long enough to actually click on any hyperlinks. The main package being dropped brings several other packages along with it, so the project as a whole will have a slightly lighter footprint than before. The existing `XssVulnerability` test cases are all preserved. Although the final result of the sanitizing changes, it can easily be seen that the results are all harmless. --- CHANGELOG.md | 4 + composer.json | 3 +- composer.lock | 901 +++++------------- src/PhpSpreadsheet/Writer/Html.php | 17 +- .../Writer/Html/HtmlCommentsTest.php | 57 +- .../Writer/Html/XssVulnerabilityTest.php | 19 +- 6 files changed, 311 insertions(+), 690 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47f32b45c5..014ce05b2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org). ## Unreleased - TBD +### BREAKING CHANGE + +- Writing of cell comments to Html will now sanitize all Html tags within the comment, so the tags will be rendered as plaintext and have no other effects when rendered. Styling can be achieved by using the Font property of of the TextRuns which make up the comment, as is already the cases for Xlsx. [PR #3957](https://github.com/PHPOffice/PhpSpreadsheet/pull/3957) + ### Added - Default Style Alignment Property (workaround for bug in non-Excel spreadsheet apps) [Issue #3918](https://github.com/PHPOffice/PhpSpreadsheet/issues/3918) [PR #3924](https://github.com/PHPOffice/PhpSpreadsheet/pull/3924) diff --git a/composer.json b/composer.json index 5a7e9164a8..2365bbe9de 100644 --- a/composer.json +++ b/composer.json @@ -84,8 +84,7 @@ "markbaker/matrix": "^3.0", "psr/http-client": "^1.0", "psr/http-factory": "^1.0", - "psr/simple-cache": "^1.0 || ^2.0 || ^3.0", - "voku/anti-xss": "^4.1" + "psr/simple-cache": "^1.0 || ^2.0 || ^3.0" }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "dev-main", diff --git a/composer.lock b/composer.lock index 89de851951..c42dc590ce 100644 --- a/composer.lock +++ b/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": "84f741787fe1d67df41a23c583e92220", + "content-hash": "c7a7ba2e1cc9ac479e06ab28916c6b80", "packages": [ { "name": "maennchen/zipstream-php", @@ -464,664 +464,6 @@ "source": "https://github.com/php-fig/simple-cache/tree/3.0.0" }, "time": "2021-10-29T13:26:27+00:00" - }, - { - "name": "symfony/polyfill-iconv", - "version": "v1.28.0", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-iconv.git", - "reference": "6de50471469b8c9afc38164452ab2b6170ee71c1" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-iconv/zipball/6de50471469b8c9afc38164452ab2b6170ee71c1", - "reference": "6de50471469b8c9afc38164452ab2b6170ee71c1", - "shasum": "" - }, - "require": { - "php": ">=7.1" - }, - "provide": { - "ext-iconv": "*" - }, - "suggest": { - "ext-iconv": "For best performance" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-main": "1.28-dev" - }, - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "files": [ - "bootstrap.php" - ], - "psr-4": { - "Symfony\\Polyfill\\Iconv\\": "" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill for the Iconv extension", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "iconv", - "polyfill", - "portable", - "shim" - ], - "support": { - "source": "https://github.com/symfony/polyfill-iconv/tree/v1.28.0" - }, - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2023-01-26T09:26:14+00:00" - }, - { - "name": "symfony/polyfill-intl-grapheme", - "version": "v1.29.0", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-intl-grapheme.git", - "reference": "32a9da87d7b3245e09ac426c83d334ae9f06f80f" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-intl-grapheme/zipball/32a9da87d7b3245e09ac426c83d334ae9f06f80f", - "reference": "32a9da87d7b3245e09ac426c83d334ae9f06f80f", - "shasum": "" - }, - "require": { - "php": ">=7.1" - }, - "suggest": { - "ext-intl": "For best performance" - }, - "type": "library", - "extra": { - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "files": [ - "bootstrap.php" - ], - "psr-4": { - "Symfony\\Polyfill\\Intl\\Grapheme\\": "" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill for intl's grapheme_* functions", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "grapheme", - "intl", - "polyfill", - "portable", - "shim" - ], - "support": { - "source": "https://github.com/symfony/polyfill-intl-grapheme/tree/v1.29.0" - }, - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2024-01-29T20:11:03+00:00" - }, - { - "name": "symfony/polyfill-intl-normalizer", - "version": "v1.29.0", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-intl-normalizer.git", - "reference": "bc45c394692b948b4d383a08d7753968bed9a83d" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-intl-normalizer/zipball/bc45c394692b948b4d383a08d7753968bed9a83d", - "reference": "bc45c394692b948b4d383a08d7753968bed9a83d", - "shasum": "" - }, - "require": { - "php": ">=7.1" - }, - "suggest": { - "ext-intl": "For best performance" - }, - "type": "library", - "extra": { - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "files": [ - "bootstrap.php" - ], - "psr-4": { - "Symfony\\Polyfill\\Intl\\Normalizer\\": "" - }, - "classmap": [ - "Resources/stubs" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill for intl's Normalizer class and related functions", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "intl", - "normalizer", - "polyfill", - "portable", - "shim" - ], - "support": { - "source": "https://github.com/symfony/polyfill-intl-normalizer/tree/v1.29.0" - }, - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2024-01-29T20:11:03+00:00" - }, - { - "name": "symfony/polyfill-mbstring", - "version": "v1.29.0", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-mbstring.git", - "reference": "9773676c8a1bb1f8d4340a62efe641cf76eda7ec" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-mbstring/zipball/9773676c8a1bb1f8d4340a62efe641cf76eda7ec", - "reference": "9773676c8a1bb1f8d4340a62efe641cf76eda7ec", - "shasum": "" - }, - "require": { - "php": ">=7.1" - }, - "provide": { - "ext-mbstring": "*" - }, - "suggest": { - "ext-mbstring": "For best performance" - }, - "type": "library", - "extra": { - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "files": [ - "bootstrap.php" - ], - "psr-4": { - "Symfony\\Polyfill\\Mbstring\\": "" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill for the Mbstring extension", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "mbstring", - "polyfill", - "portable", - "shim" - ], - "support": { - "source": "https://github.com/symfony/polyfill-mbstring/tree/v1.29.0" - }, - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2024-01-29T20:11:03+00:00" - }, - { - "name": "symfony/polyfill-php72", - "version": "v1.28.0", - "source": { - "type": "git", - "url": "https://github.com/symfony/polyfill-php72.git", - "reference": "70f4aebd92afca2f865444d30a4d2151c13c3179" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-php72/zipball/70f4aebd92afca2f865444d30a4d2151c13c3179", - "reference": "70f4aebd92afca2f865444d30a4d2151c13c3179", - "shasum": "" - }, - "require": { - "php": ">=7.1" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-main": "1.28-dev" - }, - "thanks": { - "name": "symfony/polyfill", - "url": "https://github.com/symfony/polyfill" - } - }, - "autoload": { - "files": [ - "bootstrap.php" - ], - "psr-4": { - "Symfony\\Polyfill\\Php72\\": "" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions", - "homepage": "https://symfony.com", - "keywords": [ - "compatibility", - "polyfill", - "portable", - "shim" - ], - "support": { - "source": "https://github.com/symfony/polyfill-php72/tree/v1.28.0" - }, - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2023-01-26T09:26:14+00:00" - }, - { - "name": "voku/anti-xss", - "version": "4.1.42", - "source": { - "type": "git", - "url": "https://github.com/voku/anti-xss.git", - "reference": "bca1f8607e55a3c5077483615cd93bd8f11bd675" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/voku/anti-xss/zipball/bca1f8607e55a3c5077483615cd93bd8f11bd675", - "reference": "bca1f8607e55a3c5077483615cd93bd8f11bd675", - "shasum": "" - }, - "require": { - "php": ">=7.0.0", - "voku/portable-utf8": "~6.0.2" - }, - "require-dev": { - "phpunit/phpunit": "~6.0 || ~7.0 || ~9.0" - }, - "type": "library", - "extra": { - "branch-alias": { - "dev-master": "4.1.x-dev" - } - }, - "autoload": { - "psr-4": { - "voku\\helper\\": "src/voku/helper/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "EllisLab Dev Team", - "homepage": "http://ellislab.com/" - }, - { - "name": "Lars Moelleken", - "email": "lars@moelleken.org", - "homepage": "https://www.moelleken.org/" - } - ], - "description": "anti xss-library", - "homepage": "https://github.com/voku/anti-xss", - "keywords": [ - "anti-xss", - "clean", - "security", - "xss" - ], - "support": { - "issues": "https://github.com/voku/anti-xss/issues", - "source": "https://github.com/voku/anti-xss/tree/4.1.42" - }, - "funding": [ - { - "url": "https://www.paypal.me/moelleken", - "type": "custom" - }, - { - "url": "https://github.com/voku", - "type": "github" - }, - { - "url": "https://opencollective.com/anti-xss", - "type": "open_collective" - }, - { - "url": "https://www.patreon.com/voku", - "type": "patreon" - }, - { - "url": "https://tidelift.com/funding/github/packagist/voku/anti-xss", - "type": "tidelift" - } - ], - "time": "2023-07-03T14:40:46+00:00" - }, - { - "name": "voku/portable-ascii", - "version": "2.0.1", - "source": { - "type": "git", - "url": "https://github.com/voku/portable-ascii.git", - "reference": "b56450eed252f6801410d810c8e1727224ae0743" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/voku/portable-ascii/zipball/b56450eed252f6801410d810c8e1727224ae0743", - "reference": "b56450eed252f6801410d810c8e1727224ae0743", - "shasum": "" - }, - "require": { - "php": ">=7.0.0" - }, - "require-dev": { - "phpunit/phpunit": "~6.0 || ~7.0 || ~9.0" - }, - "suggest": { - "ext-intl": "Use Intl for transliterator_transliterate() support" - }, - "type": "library", - "autoload": { - "psr-4": { - "voku\\": "src/voku/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Lars Moelleken", - "homepage": "http://www.moelleken.org/" - } - ], - "description": "Portable ASCII library - performance optimized (ascii) string functions for php.", - "homepage": "https://github.com/voku/portable-ascii", - "keywords": [ - "ascii", - "clean", - "php" - ], - "support": { - "issues": "https://github.com/voku/portable-ascii/issues", - "source": "https://github.com/voku/portable-ascii/tree/2.0.1" - }, - "funding": [ - { - "url": "https://www.paypal.me/moelleken", - "type": "custom" - }, - { - "url": "https://github.com/voku", - "type": "github" - }, - { - "url": "https://opencollective.com/portable-ascii", - "type": "open_collective" - }, - { - "url": "https://www.patreon.com/voku", - "type": "patreon" - }, - { - "url": "https://tidelift.com/funding/github/packagist/voku/portable-ascii", - "type": "tidelift" - } - ], - "time": "2022-03-08T17:03:00+00:00" - }, - { - "name": "voku/portable-utf8", - "version": "6.0.13", - "source": { - "type": "git", - "url": "https://github.com/voku/portable-utf8.git", - "reference": "b8ce36bf26593e5c2e81b1850ef0ffb299d2043f" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/voku/portable-utf8/zipball/b8ce36bf26593e5c2e81b1850ef0ffb299d2043f", - "reference": "b8ce36bf26593e5c2e81b1850ef0ffb299d2043f", - "shasum": "" - }, - "require": { - "php": ">=7.0.0", - "symfony/polyfill-iconv": "~1.0", - "symfony/polyfill-intl-grapheme": "~1.0", - "symfony/polyfill-intl-normalizer": "~1.0", - "symfony/polyfill-mbstring": "~1.0", - "symfony/polyfill-php72": "~1.0", - "voku/portable-ascii": "~2.0.0" - }, - "require-dev": { - "phpstan/phpstan": "1.9.*@dev", - "phpstan/phpstan-strict-rules": "1.4.*@dev", - "phpunit/phpunit": "~6.0 || ~7.0 || ~9.0", - "thecodingmachine/phpstan-strict-rules": "1.0.*@dev", - "voku/phpstan-rules": "3.1.*@dev" - }, - "suggest": { - "ext-ctype": "Use Ctype for e.g. hexadecimal digit detection", - "ext-fileinfo": "Use Fileinfo for better binary file detection", - "ext-iconv": "Use iconv for best performance", - "ext-intl": "Use Intl for best performance", - "ext-json": "Use JSON for string detection", - "ext-mbstring": "Use Mbstring for best performance" - }, - "type": "library", - "autoload": { - "files": [ - "bootstrap.php" - ], - "psr-4": { - "voku\\": "src/voku/" - } - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "(Apache-2.0 or GPL-2.0)" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Hamid Sarfraz", - "homepage": "http://pageconfig.com/" - }, - { - "name": "Lars Moelleken", - "homepage": "http://www.moelleken.org/" - } - ], - "description": "Portable UTF-8 library - performance optimized (unicode) string functions for php.", - "homepage": "https://github.com/voku/portable-utf8", - "keywords": [ - "UTF", - "clean", - "php", - "unicode", - "utf-8", - "utf8" - ], - "support": { - "issues": "https://github.com/voku/portable-utf8/issues", - "source": "https://github.com/voku/portable-utf8/tree/6.0.13" - }, - "funding": [ - { - "url": "https://www.paypal.me/moelleken", - "type": "custom" - }, - { - "url": "https://github.com/voku", - "type": "github" - }, - { - "url": "https://opencollective.com/portable-utf8", - "type": "open_collective" - }, - { - "url": "https://www.patreon.com/voku", - "type": "patreon" - }, - { - "url": "https://tidelift.com/funding/github/packagist/voku/portable-utf8", - "type": "tidelift" - } - ], - "time": "2023-03-08T08:35:38+00:00" } ], "packages-dev": [ @@ -4826,6 +4168,245 @@ ], "time": "2024-01-29T20:11:03+00:00" }, + { + "name": "symfony/polyfill-intl-grapheme", + "version": "v1.29.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/polyfill-intl-grapheme.git", + "reference": "32a9da87d7b3245e09ac426c83d334ae9f06f80f" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/polyfill-intl-grapheme/zipball/32a9da87d7b3245e09ac426c83d334ae9f06f80f", + "reference": "32a9da87d7b3245e09ac426c83d334ae9f06f80f", + "shasum": "" + }, + "require": { + "php": ">=7.1" + }, + "suggest": { + "ext-intl": "For best performance" + }, + "type": "library", + "extra": { + "thanks": { + "name": "symfony/polyfill", + "url": "https://github.com/symfony/polyfill" + } + }, + "autoload": { + "files": [ + "bootstrap.php" + ], + "psr-4": { + "Symfony\\Polyfill\\Intl\\Grapheme\\": "" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony polyfill for intl's grapheme_* functions", + "homepage": "https://symfony.com", + "keywords": [ + "compatibility", + "grapheme", + "intl", + "polyfill", + "portable", + "shim" + ], + "support": { + "source": "https://github.com/symfony/polyfill-intl-grapheme/tree/v1.29.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2024-01-29T20:11:03+00:00" + }, + { + "name": "symfony/polyfill-intl-normalizer", + "version": "v1.29.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/polyfill-intl-normalizer.git", + "reference": "bc45c394692b948b4d383a08d7753968bed9a83d" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/polyfill-intl-normalizer/zipball/bc45c394692b948b4d383a08d7753968bed9a83d", + "reference": "bc45c394692b948b4d383a08d7753968bed9a83d", + "shasum": "" + }, + "require": { + "php": ">=7.1" + }, + "suggest": { + "ext-intl": "For best performance" + }, + "type": "library", + "extra": { + "thanks": { + "name": "symfony/polyfill", + "url": "https://github.com/symfony/polyfill" + } + }, + "autoload": { + "files": [ + "bootstrap.php" + ], + "psr-4": { + "Symfony\\Polyfill\\Intl\\Normalizer\\": "" + }, + "classmap": [ + "Resources/stubs" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony polyfill for intl's Normalizer class and related functions", + "homepage": "https://symfony.com", + "keywords": [ + "compatibility", + "intl", + "normalizer", + "polyfill", + "portable", + "shim" + ], + "support": { + "source": "https://github.com/symfony/polyfill-intl-normalizer/tree/v1.29.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2024-01-29T20:11:03+00:00" + }, + { + "name": "symfony/polyfill-mbstring", + "version": "v1.29.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/polyfill-mbstring.git", + "reference": "9773676c8a1bb1f8d4340a62efe641cf76eda7ec" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/polyfill-mbstring/zipball/9773676c8a1bb1f8d4340a62efe641cf76eda7ec", + "reference": "9773676c8a1bb1f8d4340a62efe641cf76eda7ec", + "shasum": "" + }, + "require": { + "php": ">=7.1" + }, + "provide": { + "ext-mbstring": "*" + }, + "suggest": { + "ext-mbstring": "For best performance" + }, + "type": "library", + "extra": { + "thanks": { + "name": "symfony/polyfill", + "url": "https://github.com/symfony/polyfill" + } + }, + "autoload": { + "files": [ + "bootstrap.php" + ], + "psr-4": { + "Symfony\\Polyfill\\Mbstring\\": "" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Symfony polyfill for the Mbstring extension", + "homepage": "https://symfony.com", + "keywords": [ + "compatibility", + "mbstring", + "polyfill", + "portable", + "shim" + ], + "support": { + "source": "https://github.com/symfony/polyfill-mbstring/tree/v1.29.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2024-01-29T20:11:03+00:00" + }, { "name": "symfony/polyfill-php80", "version": "v1.29.0", @@ -5422,5 +5003,5 @@ "platform-overrides": { "php": "8.0.99" }, - "plugin-api-version": "2.6.0" + "plugin-api-version": "2.3.0" } diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index 4dcc3d6652..6176d826ec 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -28,7 +28,6 @@ use PhpOffice\PhpSpreadsheet\Worksheet\MemoryDrawing; use PhpOffice\PhpSpreadsheet\Worksheet\PageSetup; use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet; -use voku\helper\AntiXSS; class Html extends BaseWriter { @@ -1237,10 +1236,11 @@ private function generateRowCellCss(Worksheet $worksheet, string $cellAddress, i return [$cell, $cssClass, $coordinate]; } - private function generateRowCellDataValueRich(Cell $cell, string &$cellData): void + private function generateRowCellDataValueRich(RichText $richText): string { + $cellData = ''; // Loop through rich text elements - $elements = $cell->getValue()->getRichTextElements(); + $elements = $richText->getRichTextElements(); foreach ($elements as $element) { // Rich text start? if ($element instanceof Run) { @@ -1255,6 +1255,8 @@ private function generateRowCellDataValueRich(Cell $cell, string &$cellData): vo $cellData .= ''; $cellEnd = ''; } + } else { + $cellData .= ''; } // Convert UTF8 data to PCDATA @@ -1270,12 +1272,14 @@ private function generateRowCellDataValueRich(Cell $cell, string &$cellData): vo $cellData .= htmlspecialchars($cellText, Settings::htmlEntityFlags()); } } + + return nl2br($cellData); } private function generateRowCellDataValue(Worksheet $worksheet, Cell $cell, string &$cellData): void { if ($cell->getValue() instanceof RichText) { - $this->generateRowCellDataValueRich($cell, $cellData); + $cellData .= $this->generateRowCellDataValueRich($cell->getValue()); } else { $origData = $this->preCalculateFormulas ? $cell->getCalculatedValue() : $cell->getValue(); $formatCode = $worksheet->getParentOrThrow()->getCellXfByIndex($cell->getXfIndex())->getNumberFormat()->getFormatCode(); @@ -1703,11 +1707,10 @@ private function writeComment(Worksheet $worksheet, string $coordinate): string { $result = ''; if (!$this->isPdf && isset($worksheet->getComments()[$coordinate])) { - $sanitizer = new AntiXSS(); - $sanitizedString = $sanitizer->xss_clean($worksheet->getComment($coordinate)->getText()->getPlainText()); + $sanitizedString = $this->generateRowCellDataValueRich($worksheet->getComment($coordinate)->getText()); if ($sanitizedString !== '') { $result .= ''; - $result .= '
' . nl2br($sanitizedString) . '
'; + $result .= '
' . $sanitizedString . '
'; $result .= PHP_EOL; } } diff --git a/tests/PhpSpreadsheetTests/Writer/Html/HtmlCommentsTest.php b/tests/PhpSpreadsheetTests/Writer/Html/HtmlCommentsTest.php index 15378e12b1..074b351f6d 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/HtmlCommentsTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/HtmlCommentsTest.php @@ -6,12 +6,11 @@ use PhpOffice\PhpSpreadsheet\RichText\RichText; use PhpOffice\PhpSpreadsheet\Spreadsheet; +use PhpOffice\PhpSpreadsheet\Writer\Html; use PhpOffice\PhpSpreadsheetTests\Functional; class HtmlCommentsTest extends Functional\AbstractFunctional { - private Spreadsheet $spreadsheet; - public static function providerCommentRichText(): array { $valueSingle = 'I am comment.'; @@ -40,31 +39,65 @@ public static function providerCommentRichText(): array $font->setBold(true); $richMultiMixed->createText(PHP_EOL . 'comment!'); + $richMultiMixed = new RichText(); + $richMultiMixed->createText('I am' . PHP_EOL); + $font = $richMultiMixed->createTextRun('multi-line')->getFont(); + self::assertNotNull($font); + $font->setBold(true); + $richMultiMixed->createText(PHP_EOL . 'comment!'); + + $scriptTextSingle = 'I am '; + $scriptSingle = new RichText(); + $scriptSingle->createText($scriptTextSingle); + return [ - 'single line plain text' => [$plainSingle], - 'multi-line plain text' => [$plainMulti], - 'single line simple rich text' => [$richSingle], - 'multi-line simple rich text' => [$richMultiSimple], - 'multi-line mixed rich text' => [$richMultiMixed], + 'single line plain text' => [$plainSingle, '
' + . 'I am comment.
' . PHP_EOL + . 'Comment'], + 'multi-line plain text' => [$plainMulti, '
' + . 'I am
' . PHP_EOL + . 'multi-line
' . PHP_EOL + . 'comment.
' . PHP_EOL + . 'Comment'], + 'single line simple rich text' => [$richSingle, '
' + . "I am comment.
" . PHP_EOL + . 'Comment'], + 'multi-line simple rich text' => [$richMultiSimple, '
' + . "I am
" . PHP_EOL + . 'multi-line
' . PHP_EOL + . 'comment.
' . PHP_EOL + . 'Comment'], + 'multi-line mixed rich text' => [$richMultiMixed, '
I am
' . PHP_EOL + . "multi-line
" . PHP_EOL + . 'comment!
' . PHP_EOL + . 'Comment'], + 'script single' => [$scriptSingle, '
' + . 'I am <script>alert("Hello")</script>
' . PHP_EOL + . 'Comment'], ]; } /** * @dataProvider providerCommentRichText */ - public function testComments(RichText $richText): void + public function testComments(RichText $richText, string $expected): void { - $this->spreadsheet = new Spreadsheet(); + $spreadsheet = new Spreadsheet(); - $this->spreadsheet->getActiveSheet()->getCell('A1')->setValue('Comment'); + $spreadsheet->getActiveSheet()->getCell('A1')->setValue('Comment'); - $this->spreadsheet->getActiveSheet() + $spreadsheet->getActiveSheet() ->getComment('A1') ->setText($richText); + $writer = new Html($spreadsheet); + $output = $writer->generateHtmlAll(); + self::assertStringContainsString($expected, $output); - $reloadedSpreadsheet = $this->writeAndReload($this->spreadsheet, 'Html'); + $reloadedSpreadsheet = $this->writeAndReload($spreadsheet, 'Html'); + $spreadsheet->disconnectWorksheets(); $actual = $reloadedSpreadsheet->getActiveSheet()->getComment('A1')->getText()->getPlainText(); self::assertSame($richText->getPlainText(), $actual); + $reloadedSpreadsheet->disconnectWorksheets(); } } diff --git a/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php b/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php index 121257103b..28716b8ad8 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php @@ -15,15 +15,16 @@ public static function providerXssRichText(): array { return [ 'basic text no problem' => ['Hello, I am safely viewing your site', 'Hello, I am safely viewing your site'], - 'link no problem' => ["Google is here", "Google is here"], - 'script tag' => ["Hello, I am trying to your site", 'Hello, I am trying to your site'], - 'javascript tag no hex' => ["CLICK", "CLICK"], - 'javascript tag' => ["CLICK", "CLICK"], - 'with unicode' => ['CLICK', 'CLICK'], - 'inline css' => ['
  • ', '
  • '], - 'char value chevron' => ["\x3cscript src=http://www.example.com/malicious-code.js\x3e\x3c/script\x3e"], - 'hexadecimal html' => ['', ''], - 'iframe' => ['', '<iframe width="560" height="315" src="https://www.youtube.com/embed/whatever?rel=0&controls=0&showinfo=0" frameborder="0" allowfullscreen></iframe>'], + 'link eliminated' => ["Google is here", "<a href='Visit Google'>Google is here</a>"], + 'script tag' => ["Hello, I am trying to your site", "Hello, I am trying to <script>alert('Hack');</script> your site"], + 'script tag with quotes' => ['Hello, I am trying to your site', 'Hello, I am trying to <script>alert("Hack");</script> your site'], + 'javascript tag no hex' => ["CLICK", "<a href='javascript:alert(1)'>CLICK</a>"], + 'javascript tag' => ["CLICK", "<a href='&#x2000;javascript:alert(1)'>CLICK</a>"], + 'with unicode' => ['CLICK', '<a href="\\u0001java\\u0003script:alert(1)">CLICK</a>'], + 'inline css' => ['
  • ', '<li style="list-style-image: url(javascript:alert(0))">'], + 'char value chevron' => ["\x3cscript src=http://www.example.com/malicious-code.js\x3e\x3c/script\x3e", '<script src=http://www.example.com/malicious-code.js></script>'], + 'hexadecimal html' => ['', '<IMG SRC=&#106&#x61&#x76&#x61&#x73&#109&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29>'], + 'iframe' => ['', '<iframe width="560" onclick="alert(\'xss\')" height="315" src="https://www.youtube.com/embed/whatever?rel=0&controls=0&showinfo=0" frameborder="0" allowfullscreen></iframe>'], ]; } From a9ac138300ae75e1bba1eaa981e5666d73b26835 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 21 Mar 2024 08:53:13 -0700 Subject: [PATCH 2/4] Migration Aid Provide a means for potentially affected users to test if new logic is in effect so that they can update code beforehand. --- src/PhpSpreadsheet/Writer/Html.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index 6176d826ec..ac14e31520 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -35,6 +35,21 @@ class Html extends BaseWriter private const DEFAULT_CELL_WIDTH_PIXELS = 56; + /** + * Migration aid to tell if html tags will be treated as plaintext in comments. + * if ( + * defined( + * \PhpOffice\PhpSpreadsheet\Writer\Html::class + * . '::COMMENT_HTML_TAGS_PLAINTEXT' + * ) + * ) { + * new logic with styling in TextRun elements + * } else { + * old logic with styling via Html tags + * }. + */ + public const COMMENT_HTML_TAGS_PLAINTEXT = true; + /** * Spreadsheet object. */ From c695d5ca8b118943bd018d18444eae8cdeff281e Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:01:13 -0700 Subject: [PATCH 3/4] Minor Correction Sanitize font name. --- src/PhpSpreadsheet/Writer/Html.php | 2 +- .../Writer/Html/XssVulnerabilityTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/PhpSpreadsheet/Writer/Html.php b/src/PhpSpreadsheet/Writer/Html.php index ac14e31520..01733205e8 100644 --- a/src/PhpSpreadsheet/Writer/Html.php +++ b/src/PhpSpreadsheet/Writer/Html.php @@ -1039,7 +1039,7 @@ private function createCSSStyleFont(Font $font): array } $css['color'] = '#' . $font->getColor()->getRGB(); - $css['font-family'] = '\'' . $font->getName() . '\''; + $css['font-family'] = '\'' . htmlspecialchars((string) $font->getName(), ENT_QUOTES) . '\''; $css['font-size'] = $font->getSize() . 'pt'; return $css; diff --git a/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php b/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php index 28716b8ad8..1d8bcbb4b2 100644 --- a/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php +++ b/tests/PhpSpreadsheetTests/Writer/Html/XssVulnerabilityTest.php @@ -59,4 +59,21 @@ public function testXssInComment(string $xssTextString, ?string $expected = null self::assertStringContainsString($expected, $verify); $spreadsheet->disconnectWorksheets(); } + + public function testXssInFontName(): void + { + $spreadsheet = new Spreadsheet(); + $sheet = $spreadsheet->getActiveSheet(); + $sheet->getCell('A1')->setValue('here'); + $used = 'Calibri