[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=16292756#comment-16292756 ]

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

Here are the most relevant parts of the discussion held in the OFBiz private security at the time of the disclosure by Loris Nardo. I think they can help here.

----
Jacopo wrote

My suggestion is to fix this by editing the service definition for service "updateProductContent" by setting allow-html="safe" on a few fields (small/medium/large images and a few others).
However there are some fields where embedding html should be allowed and this will still allow authenticated malicious backend users to cause similar problems: for this reason I don't think we should treat this report as an official security vulnerability (with CVE etc...): all in all an authenticated malicious user may cause any kind of troubles in the system.

So I would suggest to treat it as an enhancement to security rather than a security vulnerability.
----
Gregory answered to Jacopo:

If there are fields where html is needed and we can authorize a specific set of html tags the following solution can fit.
All the content is html encoded and then a regex is applied to replace all encoded tags by the authorized html tag.

Example:
{code}
<script> -> html encoded -> &lt;script&gt;
<br> -> html encoded >  &lt;br&gt; -> search  and replace ("&lt;br&gt;", "<br>") -> <br>
{code}

By doing this we only replace authorized tags. And it might be configured through a file.

Otherwise we can also explicitly say that there dangerous fields that can lead to XSS.

----
I answered to Gregory:

We currently use UtilCodec.checkStringForHtmlStrictNone() method to validate the field content, and reject "<" and ">" after canonisation.

I did not look into all details yet, but as just sent to Jacopo ^1^, I suggest we use the same type of approach I used for validating the content of generated forms used in Flexible Reports.
The idea is to reintroduce the "safe" mode and rely on PERMISSIVE_POLICY as defined in UtilCodec.HtmlEncoder() or a more specific policy for safe.

As I suggested in my 1st comment at OFBIZ-9373, where it give some more details, we could even extend the principle to allow users to define their own policies.
In the meantime, it seems to me, your solution would be OK because we currently only reject "<" and ">" despite the comment of checkStringForHtmlStrictNone()

{quote}
Does not allow various characters (after canonicalization), including
"&lt;", "&gt;", "&amp;" (if not followed by a space), and "%" (if not
followed by a space).
{quote}

I did not check code on that point...
----
^1^ Hi Jacopo,

Actually, as explained in commit comment, since r1536324 there is no longer "safe" mode for allow-html.
To keep the previous behaviour, I then decided to use "any" to replace "safe" because "safe" was doing the same than "any" but logging a confusing message in log.
Maybe I should have used "none" then.

So, as you said in commit comment and did with r1759065, for now we can only resort to "none".

While looking at OFBIZ-9373 I had an idea and commented there (1st comment).
----
Gregory added
I think it is important to highlight that HTML is interpreted in these fields.
Because more than a vulnerability the entire display of the page can be broken if one put some HTML content without knowing it.

> 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
>
> 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
(v6.4.14#64029)