search

SAP / openui5

OpenUI5 lets you build enterprise-ready web applications, responsive to all devices, running on almost any browser of your choice.
http://openui5.org
Apache License 2.0
2.95k stars 1.24k forks source link

sanitizeHTML wrong return result #3771

Closed nnaydenow closed 1 year ago

nnaydenow commented 1 year ago

OpenUI5 version: 1.115.0

Browser/version (+device/version): Google Chrome Version 113.0.5672.126

URL (minimal example if possible): https://jsbin.com/coqupokose/edit?html,output

Steps to reproduce the problem: We are using the sap/base/security/sanitizeHTML to sanitize our string in our product and found an issue for this function with style 'font-weight:900'. HTML sanitiser will replace the number string '900' with '' before checking the correct policy for font-weight related cssSchema. This function works well with non-number style like 'font-weight: bold', 'font-weight: bolder', etc.

What is the expected result? HTML sanitiser to return <p><span style="font-weight: 900">test</span> text</p>

What happens instead? HTML sanitiser returns <p><span style="font-weight:">test</span> text</p>

Any other information? (attach screenshot if possible) This issue is related to issue https://github.com/SAP/ui5-webcomponents/issues/7080.

PetyaMarkovaBogdanova commented 1 year ago

Hello @nnaydenow , Thank you for sharing this finding. I've created an internal incident 2380058937. The status of the issue will be updated here in GitHub. Regards, Petya Markova. (UI5 Dispatcher)

codeworrior commented 1 year ago

The sanitizer had indeed listed the standard numerical font weights like 100, 200, ... 800, 900 etc. as allowed enum values for the font-weight CSS property (see here). But before checking against that enum, the sanitizer took the wrong decision to check the value as a numeric value instead (something that the sanitizer's metadata for the font-weight property don't allow).

This has been fixed with https://github.com/SAP/openui5/commit/e8833e66b92fd5bcedf9d8fbe601799e41d25eff and the standard numeric values are now accepted.

However, the sanitizer does still not support the more fine grained values that the CSS Fonts Level 4 spec allows. We have no plans to enhance the caja-html-sanitizer to support this. If this is required in some scenario, using a 3rdparty sanitizer is currently the only possible solution.