You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by "Jacques Le Roux (JIRA)" <ji...@apache.org> on 2015/10/13 02:42:05 UTC

[jira] [Updated] (OFBIZ-6669) Possible static XSS issue with Content

     [ https://issues.apache.org/jira/browse/OFBIZ-6669?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jacques Le Roux updated OFBIZ-6669:
-----------------------------------
    Description: 
I found a possible XSS attack through *ContentWrapper.java and ContentWorker itself.

Note that in supported releases it's hard to exploit, it's a Stored XSS https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting which means you need 1st to somehow inject exploiting code in the DB.

Issues in *ContentWrapper.java have already been fixed by changing the ContentWrapper interface
from
{code}
    public interface ContentWrapper {
        public StringUtil.StringWrapper get(String contentTypeId);
    }
{code}
to
{code}
    public interface ContentWrapper {
        public StringUtil.StringWrapper get(String contentTypeId, String encoderType) {
    }
{code}

And changing the Category, Party, Product, ProductPromo, ProductConfigItem and WorkEffort ContentWrapperS accordingly. This means to use 2 types of encoderTypes: "html" and "url".
The "html"  encoderType will be used for all ProductContentTypes but those who contain URL in their ContentTypeIdS (actually end with, "_URL") which will use "url" encoderType.
It concerns not only the get() method but also methods like getPartyContentAsText(), getProductContentAsText(), etc.

It seems a big change but it's straightforward. It's now complete after following commits in revisions (I hope I did not miss to report):
trunk 1705329 1705417 1705427 1705532 1706159 1706162 1707857
and related backports in R14.12 1705331 1705418 1705428 1705533 1706160 1706163 1707858


I have also committed a fix for ContentWorker. For that I have added owasp-java-html-sanitizer-r239.jar and put a "content.sanitize=true" property in content.properties with some explanations. The reason I put this property is because the sanitizer does some (safe) changes which might be unwanted in a context where you are "sure" no one can inject/exploit your DB.

Here is for instance the changes the sanitizer does when rendering cmssite
{code}
@@ -19,7 +19,7 @@
 <body>


-            <div id="header">
+            <div>
                 <h1>This is the header!</h1>
             </div>

@@ -27,34 +27,26 @@

             <div>
               <h1>Welcome to the CmsSite Home page.</h1>
-              <center><table width="350"><tr><td>
+
               <p>
               This is a site to demonstrate the CMS capabilities of OFBiz. Its basic function is the editing of website text
               inside a browser. If you want to edit the text you are reading now, logon to the backend system, select the content component
-              click on 'cmssite' in the website list and ten click on the 'cms' button. There you see on the left hand side the tree of this website.
-              If you click on 'homepage' then you can edit the content of this page at the box in the r
+              click on &#39;cmssite&#39; in the website list and ten click on the &#39;cms&#39; button. There you see on the left hand side the tree of this website.
+              If you click on &#39;homepage&#39; then you can edit the content of this page at the box in the r
               </p>
               <p>
               This is only the basic function of the CMS which is part of the content component. The content component is actually more than a
               CMS it can also handle documents pretty well. An example is the apache OFBiz document you can see when you click on the last option in the list below.
-              <p>
-              </td></tr></table></center>
-              <ul>
-                <li><a href="/cmssite/cms/CMSS_DEMO_PAGE1">Demo Page 1 - Hard Coded Link</a></div>
-                <li><a href="/cmssite/cms/CMSS_PPOINT/demoPage1">Demo Page 1 - Hard Coded Link using the Sub-Content Pattern</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_PAGE1;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page 1 - Dynamic Link</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_SCREEN;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with screen widget and screen decorator</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_BLOG;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with blog using screen decorator</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_TPL_DATA;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with an xml resource formatted with a template ftl resource</a></li>
-                <li><a href="/cmssite/cms/PUBLIC_DOCS;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">The ofbiz public documents</a></li>
-              </ul>
+              </p><p>
+              </p>
+              <ul><li><a href="/cmssite/cms/CMSS_DEMO_PAGE1" rel="nofollow">Demo Page 1 - Hard Coded Link</a>
+                </li><li><a href="/cmssite/cms/CMSS_PPOINT/demoPage1" rel="nofollow">Demo Page 1 - Hard Coded Link using the Sub-Content Pattern</a></li><li><a href="/cmssite/cms/CMSS_DEMO_PAGE1" rel="nofollow">Demo Page 1 - Dynamic Link</a></li><li><a href="/cmssite/cms/CMSS_DEMO_SCREEN" rel="nofollow">Demo Page with screen widget and screen decorator</a></li><li><a href="/cmssite/cms/CMSS_DEMO_BLOG" rel="nofollow">Demo Page with blog using screen decorator</a></li><li><a href="/cmssite/cms/CMSS_DEMO_TPL_DATA" rel="nofollow">Demo Page with an xml resource formatted with a template ftl resource</a></li><li><a href="/cmssite/cms/PUBLIC_DOCS" rel="nofollow">The ofbiz public documents</a></li></ul>
             </div>


-
-            <div id="footer">
-                <h4>This is the footer!</h4>
+            <div>
+
             </div>
-            </body>
-            </html>
+
+
{code}
I wonder why it removes the ids, "<center><table" and ending </body> and </html>, but those guys know much more about XSS exploitation than me. As explained at https://www.owasp.org/index.php/OWASP_Java_HTML_Sanitizer_Project :
 * Actively maintained by Mike Samuel from Google's AppSec team!
* Passing 95+% of AntiSamy's unit tests plus many more.
* This is code from the Caja project that was donated by Google. It is rather high performance and low memory utilization.

Note that this does not affect the *ContentWrapper.java classes where we use OWASP encoding and not sanitizer. The reason we need the sanitizer here is because we are no only handling content but also HTML code...

  was:
I found a possible XSS attack through *ContentWrapper.java and ContentWorker itself.

Note that in supported releases it's hard to exploit, it's a Stored XSS https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting which means you need 1st to somehow inject exploiting code in the DB.

Issues in *ContentWrapper.java have already been fixed by changing the ContentWrapper interface
from
{code}
    public interface ContentWrapper {
        public StringUtil.StringWrapper get(String contentTypeId);
    }
{code}
to
{code}
    public interface ContentWrapper {
        public StringUtil.StringWrapper get(String contentTypeId, String encoderType) {
    }
{code}

And changing the Category, Party, Product, ProductPromo, ProductConfigItem and WorkEffort ContentWrapperS accordingly. This means to use 2 types of encoderTypes: "html" and "url".
The "html"  encoderType will be used for all ProductContentTypes but those who contain URL in their ContentTypeIdS (actually end with, "_URL") which will use "url" encoderType.
It concerns not only the get() method but also methods like getPartyContentAsText(), getProductContentAsText(), etc.

It seems a big change but it's straightforward. It's now complete after following commits in revisions (I hope I did not miss to report):
trunk 1705329 1705417 1705427 1705532 1706159 1706162 1707857
and related backports in R14.12 1705331 1705418 1705428 1705533 1706160 1706163 1707858


I have also committed a fix for ContentWorker. For that I have added owasp-java-html-sanitizer-r239.jar and put a "content.sanitize=true" property in content.properties with some explanations. The reason I put this property is because the sanitizer does some (safe) changes which might be unwanted in a context where you are "sure" no one can inject/exploit your DB.

Here is for instance the changes the sanitizer does when rendering cmssite
{code}
@@ -19,7 +19,7 @@
 <body>


-            <div id="header">
+            <div>
                 <h1>This is the header!</h1>
             </div>

@@ -27,34 +27,26 @@

             <div>
               <h1>Welcome to the CmsSite Home page.</h1>
-              <center><table width="350"><tr><td>
+
               <p>
               This is a site to demonstrate the CMS capabilities of OFBiz. Its basic function is the editing of website text
               inside a browser. If you want to edit the text you are reading now, logon to the backend system, select the content component
-              click on 'cmssite' in the website list and ten click on the 'cms' button. There you see on the left hand side the tree of this website.
-              If you click on 'homepage' then you can edit the content of this page at the box in the r
+              click on &#39;cmssite&#39; in the website list and ten click on the &#39;cms&#39; button. There you see on the left hand side the tree of this website.
+              If you click on &#39;homepage&#39; then you can edit the content of this page at the box in the r
               </p>
               <p>
               This is only the basic function of the CMS which is part of the content component. The content component is actually more than a
               CMS it can also handle documents pretty well. An example is the apache OFBiz document you can see when you click on the last option in the list below.
-              <p>
-              </td></tr></table></center>
-              <ul>
-                <li><a href="/cmssite/cms/CMSS_DEMO_PAGE1">Demo Page 1 - Hard Coded Link</a></div>
-                <li><a href="/cmssite/cms/CMSS_PPOINT/demoPage1">Demo Page 1 - Hard Coded Link using the Sub-Content Pattern</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_PAGE1;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page 1 - Dynamic Link</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_SCREEN;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with screen widget and screen decorator</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_BLOG;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with blog using screen decorator</a></li>
-                <li><a href="/cmssite/cms/CMSS_DEMO_TPL_DATA;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with an xml resource formatted with a template ftl resource</a></li>
-                <li><a href="/cmssite/cms/PUBLIC_DOCS;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">The ofbiz public documents</a></li>
-              </ul>
+              </p><p>
+              </p>
+              <ul><li><a href="/cmssite/cms/CMSS_DEMO_PAGE1" rel="nofollow">Demo Page 1 - Hard Coded Link</a>
+                </li><li><a href="/cmssite/cms/CMSS_PPOINT/demoPage1" rel="nofollow">Demo Page 1 - Hard Coded Link using the Sub-Content Pattern</a></li><li><a href="/cmssite/cms/CMSS_DEMO_PAGE1" rel="nofollow">Demo Page 1 - Dynamic Link</a></li><li><a href="/cmssite/cms/CMSS_DEMO_SCREEN" rel="nofollow">Demo Page with screen widget and screen decorator</a></li><li><a href="/cmssite/cms/CMSS_DEMO_BLOG" rel="nofollow">Demo Page with blog using screen decorator</a></li><li><a href="/cmssite/cms/CMSS_DEMO_TPL_DATA" rel="nofollow">Demo Page with an xml resource formatted with a template ftl resource</a></li><li><a href="/cmssite/cms/PUBLIC_DOCS" rel="nofollow">The ofbiz public documents</a></li></ul>
             </div>


-
-            <div id="footer">
-                <h4>This is the footer!</h4>
+            <div>
+
             </div>
-            </body>
-            </html>
+
+
{code}
I wonder why it removes the ids, "<center><table" and ending </body> and </html>, but those guys know much more about XSS exploitation than me. As explained at https://www.owasp.org/index.php/OWASP_Java_HTML_Sanitizer_Project :
 * Actively maintained by Mike Samuel from Google's AppSec team!
* Passing 95+% of AntiSamy's unit tests plus many more.
* This is code from the Caja project that was donated by Google. It is rather high performance and low memory utilization.


> Possible static XSS issue with Content
> --------------------------------------
>
>                 Key: OFBIZ-6669
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-6669
>             Project: OFBiz
>          Issue Type: Bug
>          Components: content, order, party, product, workeffort
>    Affects Versions: Release Branch 12.04, Release Branch 13.07, Release Branch 14.12, Trunk
>            Reporter: Jacques Le Roux
>            Assignee: Jacques Le Roux
>             Fix For: 14.12.01, Upcoming Branch
>
>
> I found a possible XSS attack through *ContentWrapper.java and ContentWorker itself.
> Note that in supported releases it's hard to exploit, it's a Stored XSS https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting which means you need 1st to somehow inject exploiting code in the DB.
> Issues in *ContentWrapper.java have already been fixed by changing the ContentWrapper interface
> from
> {code}
>     public interface ContentWrapper {
>         public StringUtil.StringWrapper get(String contentTypeId);
>     }
> {code}
> to
> {code}
>     public interface ContentWrapper {
>         public StringUtil.StringWrapper get(String contentTypeId, String encoderType) {
>     }
> {code}
> And changing the Category, Party, Product, ProductPromo, ProductConfigItem and WorkEffort ContentWrapperS accordingly. This means to use 2 types of encoderTypes: "html" and "url".
> The "html"  encoderType will be used for all ProductContentTypes but those who contain URL in their ContentTypeIdS (actually end with, "_URL") which will use "url" encoderType.
> It concerns not only the get() method but also methods like getPartyContentAsText(), getProductContentAsText(), etc.
> It seems a big change but it's straightforward. It's now complete after following commits in revisions (I hope I did not miss to report):
> trunk 1705329 1705417 1705427 1705532 1706159 1706162 1707857
> and related backports in R14.12 1705331 1705418 1705428 1705533 1706160 1706163 1707858
> I have also committed a fix for ContentWorker. For that I have added owasp-java-html-sanitizer-r239.jar and put a "content.sanitize=true" property in content.properties with some explanations. The reason I put this property is because the sanitizer does some (safe) changes which might be unwanted in a context where you are "sure" no one can inject/exploit your DB.
> Here is for instance the changes the sanitizer does when rendering cmssite
> {code}
> @@ -19,7 +19,7 @@
>  <body>
> -            <div id="header">
> +            <div>
>                  <h1>This is the header!</h1>
>              </div>
> @@ -27,34 +27,26 @@
>              <div>
>                <h1>Welcome to the CmsSite Home page.</h1>
> -              <center><table width="350"><tr><td>
> +
>                <p>
>                This is a site to demonstrate the CMS capabilities of OFBiz. Its basic function is the editing of website text
>                inside a browser. If you want to edit the text you are reading now, logon to the backend system, select the content component
> -              click on 'cmssite' in the website list and ten click on the 'cms' button. There you see on the left hand side the tree of this website.
> -              If you click on 'homepage' then you can edit the content of this page at the box in the r
> +              click on &#39;cmssite&#39; in the website list and ten click on the &#39;cms&#39; button. There you see on the left hand side the tree of this website.
> +              If you click on &#39;homepage&#39; then you can edit the content of this page at the box in the r
>                </p>
>                <p>
>                This is only the basic function of the CMS which is part of the content component. The content component is actually more than a
>                CMS it can also handle documents pretty well. An example is the apache OFBiz document you can see when you click on the last option in the list below.
> -              <p>
> -              </td></tr></table></center>
> -              <ul>
> -                <li><a href="/cmssite/cms/CMSS_DEMO_PAGE1">Demo Page 1 - Hard Coded Link</a></div>
> -                <li><a href="/cmssite/cms/CMSS_PPOINT/demoPage1">Demo Page 1 - Hard Coded Link using the Sub-Content Pattern</a></li>
> -                <li><a href="/cmssite/cms/CMSS_DEMO_PAGE1;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page 1 - Dynamic Link</a></li>
> -                <li><a href="/cmssite/cms/CMSS_DEMO_SCREEN;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with screen widget and screen decorator</a></li>
> -                <li><a href="/cmssite/cms/CMSS_DEMO_BLOG;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with blog using screen decorator</a></li>
> -                <li><a href="/cmssite/cms/CMSS_DEMO_TPL_DATA;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">Demo Page with an xml resource formatted with a template ftl resource</a></li>
> -                <li><a href="/cmssite/cms/PUBLIC_DOCS;jsessionid=014BD837D7FFB6E0F8CB31AAF35092A0.jvm1">The ofbiz public documents</a></li>
> -              </ul>
> +              </p><p>
> +              </p>
> +              <ul><li><a href="/cmssite/cms/CMSS_DEMO_PAGE1" rel="nofollow">Demo Page 1 - Hard Coded Link</a>
> +                </li><li><a href="/cmssite/cms/CMSS_PPOINT/demoPage1" rel="nofollow">Demo Page 1 - Hard Coded Link using the Sub-Content Pattern</a></li><li><a href="/cmssite/cms/CMSS_DEMO_PAGE1" rel="nofollow">Demo Page 1 - Dynamic Link</a></li><li><a href="/cmssite/cms/CMSS_DEMO_SCREEN" rel="nofollow">Demo Page with screen widget and screen decorator</a></li><li><a href="/cmssite/cms/CMSS_DEMO_BLOG" rel="nofollow">Demo Page with blog using screen decorator</a></li><li><a href="/cmssite/cms/CMSS_DEMO_TPL_DATA" rel="nofollow">Demo Page with an xml resource formatted with a template ftl resource</a></li><li><a href="/cmssite/cms/PUBLIC_DOCS" rel="nofollow">The ofbiz public documents</a></li></ul>
>              </div>
> -
> -            <div id="footer">
> -                <h4>This is the footer!</h4>
> +            <div>
> +
>              </div>
> -            </body>
> -            </html>
> +
> +
> {code}
> I wonder why it removes the ids, "<center><table" and ending </body> and </html>, but those guys know much more about XSS exploitation than me. As explained at https://www.owasp.org/index.php/OWASP_Java_HTML_Sanitizer_Project :
>  * Actively maintained by Mike Samuel from Google's AppSec team!
> * Passing 95+% of AntiSamy's unit tests plus many more.
> * This is code from the Caja project that was donated by Google. It is rather high performance and low memory utilization.
> Note that this does not affect the *ContentWrapper.java classes where we use OWASP encoding and not sanitizer. The reason we need the sanitizer here is because we are no only handling content but also HTML code...



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)