[jira] [Commented] (OFBIZ-10054) Product content management screen doesn't validate trusted users' input

[Nicolas Malin (Jira)]

    [ https://issues.apache.org/jira/browse/OFBIZ-10054?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16836110#comment-16836110 ]

Jacques Le Roux commented on OFBIZ-10054:
-----------------------------------------

Actually there are 2 ways used in OFBiz:
 # To prevent saving stored XSS scriptings in DB we reject them before. This is achieved with UtilCodec.checkStringForHtmlStrictNone(). [Most of the possible XSS attacks rely on the less-than (<) and greater-than (>) symbols|https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet]. But as shown with the current issue [there are other types of possibles attacks|https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#Event_Handlers].
 # Filter HTML texts and remove the unwanted parts. This is done using policies with HtmlEncoder::sanitize. The default policy is not much permissive. Since OFBIZ-10187 it's easier to create own more permissive policies. An [example inspired by eBay is available OOTB.|https://www.owasp.org/index.php/Category:OWASP_AntiSamy_Project#Stage_2_-_Choosing_a_base_policy_file]. To be safer a [policy inspired by Slashdot|https://github.com/andresriancho/owaspantisamy/blob/master/Java/antisamy-smoketest/src/main/webapp/WEB-INF/policies/antisamy-slashdot.xml] could be used. Anyway it's up to you... I should note here though that currently the AntiSamy API is not used in OFBiz. This is something that still need to be clarified with the authors of OFBIZ-10187. Maybe it was easier for them to adapt from XML to Java...

These 2 ways (reject or filter) are somehow discussed here: [https://github.com/OWASP/java-html-sanitizer/blob/master/docs/html-validation.md]

Anyway, my proposition of using HtmlEncoder::sanitize inside UtilCodec.checkStringForHtmlStrictNone() was wrong. Because they don't achieve the same goal. One rejects, the other modifies, with the hope to make the result safer (can't be 100% guaranteed).

Greg's solution don't work either for the same reason. With UtilCodec.checkStringForHtmlStrictNone(), we need to reject not change.

I have committed a solution which rejects any js event in:
trunk r1858965
R18 r1858966
R17 r1858967
R16 r1858969

According to https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet there are 2 other types of cases which are not covered by rejecting less-than (<) and greater-than (>) symbols:
# [US-ASCII_encoding|https://www.owasp.org/index.php/XSS_Filter_Evasion_Cheat_Sheet#US-ASCII_encoding]
# Flash

As I commented in code for:
# All our Tomcat connectors use UTF-8, so not a problem
# We don't care about Flash now rather deprecated in last versions of major browsers. And actually we don't use Flash OOTB at all

> Product content management screen doesn't validate trusted users' input
> -----------------------------------------------------------------------
>
>                 Key: OFBIZ-10054
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-10054
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: product
>    Affects Versions: Trunk, Release Branch 16.11
>            Reporter: Jacopo Cappellato
>            Assignee: Jacques Le Roux
>            Priority: Major
>
> Steps to recreate:
> 1) go to (authenticate with admin/ofbiz):
> https://localhost:8443/catalog/control/EditProductContent?productId=WG-1111
> 2) set the content of the field labeled "Large Image" to:
> non_existent.foo&quot; onerror=&quot;alert(&apos;Hi!&apos;);
> 3) visit the url:
> https://localhost:8443/ecommerce/control/product?product_id=WG-1111
> A popup message will appear with the "Hi!".
> Thanks to Loris Nardo for the report.

--
This message was sent by Atlassian JIRA
(v7.6.3#76005)