OFBiz
  1. OFBiz
  2. OFBIZ-178

Cross site scripting vulnerability in Forum

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Trunk
    • Fix Version/s: Trunk
    • Labels:
      None

      Description

      Currently HTML tags are filtered from forum messages by client side javascript (whyzzywig.js). If JavaScript is turned off (or local webproxy is used to filter or change the script), then user can post a forum message containing any HTML code, including <script> tags, e.g. <script>alert('test');</script>

      This is classic cross site scripting problem with all the consequences (e.g. writing scripts to steal active cookies).

      Also, currently a lot is supplied as hidden fields, which probably means that user could change that text. I have not checked that, but as there are fields like dataResourceTypeId, contentTypeId then probably user can create any type of content.
      <input type="hidden" name="VIEW_INDEX"/>
      <input type="hidden" name="threadView"/>
      <input type="hidden" name="forumGroupId"/>
      <input type="hidden" name="dataResourceTypeId" value="ELECTRONIC_TEXT"/>
      <input type="hidden" name="forumId" value="ASK"/>
      <input type="hidden" name="contentName" value="New thread/message/response"/>
      <input type="hidden" name="contentTypeId" value="DOCUMENT"/>
      <input type="hidden" name="ownerContentId" value="ASK"/>
      <input type="hidden" name="contentIdTo" value="10007"/>

      <input type="hidden" name="contentAssocTypeId" value="RESPONSE"/>

        Issue Links

          Activity

          Hide
          Jacques Le Roux added a comment -

          Fixed by recent security efforts (though the message is not clear when trying to inject in forum body, title is ok (std input field vs content field)

          Show
          Jacques Le Roux added a comment - Fixed by recent security efforts (though the message is not clear when trying to inject in forum body, title is ok (std input field vs content field)
          Hide
          Jacques Le Roux added a comment -

          Interesting use cases in OFBIZ-1476 (and link to http://www.bindshell.net/)

          Show
          Jacques Le Roux added a comment - Interesting use cases in OFBIZ-1476 (and link to http://www.bindshell.net/ )
          Hide
          Jacques Le Roux added a comment -

          Please see my comments in OFBIZ-260

          Show
          Jacques Le Roux added a comment - Please see my comments in OFBIZ-260
          Hide
          Jacques Le Roux added a comment -

          Eriks,

          I agree that this has to be corrected from the server side and I vote for this issue.

          Si,

          Yes I think that this is the place where it might go. If I have well understand that means converting some parts of the code of whizzywig.js. into java and put it in DataServices.createDataResourceMethod. Are you ready do to that work Eriks ?

          Show
          Jacques Le Roux added a comment - Eriks, I agree that this has to be corrected from the server side and I vote for this issue. Si, Yes I think that this is the place where it might go. If I have well understand that means converting some parts of the code of whizzywig.js. into java and put it in DataServices.createDataResourceMethod. Are you ready do to that work Eriks ?
          Hide
          Si Chen added a comment -

          So are you saying that createDataResource or whatever it is that is storing the html content should filter out script tags?

          Show
          Si Chen added a comment - So are you saying that createDataResource or whatever it is that is storing the html content should filter out script tags?
          Hide
          Jacques Le Roux added a comment -

          Mmm, this was a problem with hsql, it's all right with PostGres. I like to work with hsql for its speed, but I think I will return to PostGres...

          Show
          Jacques Le Roux added a comment - Mmm, this was a problem with hsql, it's all right with PostGres. I like to work with hsql for its speed, but I think I will return to PostGres...
          Hide
          Jacques Le Roux added a comment -

          I even tried after an ant clean-data (I use hsql) same error, will with an ant clean-all (but it seems useless).

          Show
          Jacques Le Roux added a comment - I even tried after an ant clean-data (I use hsql) same error, will with an ant clean-all (but it seems useless).
          Hide
          Eriks Dobelis added a comment -

          Si,

          It cannot be fixed in whizzywyg.js (it does correct filtering already now), because when JavaScript is turned off in the browser whizzywyg is not used at all. It has to be done on server side, because all client side controls can be easily manipulated by malitous user.

          Show
          Eriks Dobelis added a comment - Si, It cannot be fixed in whizzywyg.js (it does correct filtering already now), because when JavaScript is turned off in the browser whizzywyg is not used at all. It has to be done on server side, because all client side controls can be easily manipulated by malitous user.
          Hide
          Si Chen added a comment -

          Erik,

          I think then the filtering of HTML should be something to be fixed in whizzywyg.js and contributed back to them and then brought back into ofbiz. Maybe you should write them?

          I disagree with you about "the most clean approach would be to send to the client side only session ID (in cookie or hidden field) and to store all other data on the server side". I think the approach most consistent with the way ofbiz works would be to create a special service for managing forum postings which reuses the existing content services but with those fields embedded in them. The content manager services are fairly generic and probably meant to be used as foundation or building blocks for actual applications rather than directly hidden on html forms like this.

          Show
          Si Chen added a comment - Erik, I think then the filtering of HTML should be something to be fixed in whizzywyg.js and contributed back to them and then brought back into ofbiz. Maybe you should write them? I disagree with you about "the most clean approach would be to send to the client side only session ID (in cookie or hidden field) and to store all other data on the server side". I think the approach most consistent with the way ofbiz works would be to create a special service for managing forum postings which reuses the existing content services but with those fields embedded in them. The content manager services are fairly generic and probably meant to be used as foundation or building blocks for actual applications rather than directly hidden on html forms like this.
          Hide
          Jacques Le Roux added a comment -

          Erik,

          I tried with last svn (at this moment) but had not updated the data indeed. I just tried with data updated (ant run-install) and I get the same error. Strange because listSize is an "OUT" optionnal="false" parameter of performFindList and is present after the call of performFindList in ForumScreens.xml. Will see later...

          Show
          Jacques Le Roux added a comment - Erik, I tried with last svn (at this moment) but had not updated the data indeed. I just tried with data updated (ant run-install) and I get the same error. Strange because listSize is an "OUT" optionnal="false" parameter of performFindList and is present after the call of performFindList in ForumScreens.xml. Will see later...
          Hide
          Eriks Dobelis added a comment -

          Jacque, are you using latest version with demo data?

          Si, HTML text coming from the client should be checked to contain only those HTML tags which should be explicitely allowed (e.g. <strong> is one such tag). In all the other tags symbols like <, >, ', " should be for security reasons changed to their HTML representaion (< &gt). Basically the same operations that whizzywig.js does on the client side regarding symbol filtering should be performed also on the server side.

          Regarding hidden fields, the most clean approach would be to send to the client side only session ID (in cookie or hidden field) and to store all other data on the server side. Otherwise, the effect of manipulating all the hidden field values should be analyzed. If those are values which the client should be able to change then it is ok, but I am quite sure that client should not be able to change values of dataResourceTypeId, contentTypeId.

          Show
          Eriks Dobelis added a comment - Jacque, are you using latest version with demo data? Si, HTML text coming from the client should be checked to contain only those HTML tags which should be explicitely allowed (e.g. <strong> is one such tag). In all the other tags symbols like <, >, ', " should be for security reasons changed to their HTML representaion (< &gt). Basically the same operations that whizzywig.js does on the client side regarding symbol filtering should be performed also on the server side. Regarding hidden fields, the most clean approach would be to send to the client side only session ID (in cookie or hidden field) and to store all other data on the server side. Otherwise, the effect of manipulating all the hidden field values should be analyzed. If those are values which the client should be able to change then it is ok, but I am quite sure that client should not be able to change values of dataResourceTypeId, contentTypeId.
          Hide
          Si Chen added a comment -

          Eric,

          I see how it could be a problem – so you're saying I could turn off my javascript, insert some malicious script, and then everybody else who comes to the forum screen later could then have their sessionId, etc. stolen by my JavaScript for session hijacking?

          How should we solve it then? We still need to use JS for the forum screens, as I'm sure a lot of websites do. Do you have any suggestions, or better still--a patch?

          The issue of the fields – yes I agree, it's not very nice. There should be a wrapper field which sets all of these so that the amount in the HTML page should be kept to a minimum.

          Show
          Si Chen added a comment - Eric, I see how it could be a problem – so you're saying I could turn off my javascript, insert some malicious script, and then everybody else who comes to the forum screen later could then have their sessionId, etc. stolen by my JavaScript for session hijacking? How should we solve it then? We still need to use JS for the forum screens, as I'm sure a lot of websites do. Do you have any suggestions, or better still--a patch? The issue of the fields – yes I agree, it's not very nice. There should be a wrapper field which sets all of these so that the amount in the HTML page should be kept to a minimum.
          Hide
          Jacopo Cappellato added a comment -

          I'm not an expert in this area but maybe others could give some good advices: however it would be nice to see some fixes and security improvements in the forum stuff.

          Show
          Jacopo Cappellato added a comment - I'm not an expert in this area but maybe others could give some good advices: however it would be nice to see some fixes and security improvements in the forum stuff.
          Hide
          Jacques Le Roux added a comment -

          Eriks,

          The js file is actually whizzywig.js.

          Using last svn, I tried to load a forum from eCommerce 1st page and I got this message :

          org.ofbiz.base.util.GeneralException: Error rendering screen component://ecommerce/widget/ForumScreens.xml#Showforum: java.lang.IllegalArgumentException: Error calling service with name performFindList: org.ofbiz.service.ServiceValidationException: The following required parameter is missing: [performFindList.listSize] (Error calling service with name performFindList: org.ofbiz.service.ServiceValidationException: The following required parameter is missing: [performFindList.listSize])

          Please as I don't really need forums for now, might you take a look at this pb before ?

          TIA

          Jacques

          Show
          Jacques Le Roux added a comment - Eriks, The js file is actually whizzywig.js. Using last svn, I tried to load a forum from eCommerce 1st page and I got this message : org.ofbiz.base.util.GeneralException: Error rendering screen component://ecommerce/widget/ForumScreens.xml#Showforum : java.lang.IllegalArgumentException: Error calling service with name performFindList: org.ofbiz.service.ServiceValidationException: The following required parameter is missing: [performFindList.listSize] (Error calling service with name performFindList: org.ofbiz.service.ServiceValidationException: The following required parameter is missing: [performFindList.listSize] ) Please as I don't really need forums for now, might you take a look at this pb before ? TIA Jacques

            People

            • Assignee:
              David E. Jones
              Reporter:
              Eriks Dobelis
            • Votes:
              2 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development