Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-6669

Possible stored XSS issue with Content

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: Release Branch 12.04, Release Branch 13.07, Release Branch 14.12, Trunk
    • Fix Version/s: 14.12.01, 16.11.01
    • Component/s: content, order, party, product, workeffort
    • Labels:
      None

      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

          public interface ContentWrapper {
              public StringUtil.StringWrapper get(String contentTypeId);
          }
      

      to

          public interface ContentWrapper {
              public StringUtil.StringWrapper get(String contentTypeId, String encoderType) {
          }
      

      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 1708930
      and related backports in R14.12 1705331 1705418 1705428 1705533 1706160 1706163 1707858 1708931

      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

      @@ -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>
      +
      +
      

      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...

        Attachments

        1. OFBIZ-6669.patch
          16 kB
          Jacques Le Roux
        2. OFBIZ-6669.patch
          15 kB
          Jacques Le Roux

          Issue Links

            Activity

              People

              • Assignee:
                jacques.le.roux Jacques Le Roux
                Reporter:
                jacques.le.roux Jacques Le Roux
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: