OFBiz
  1. OFBiz
  2. OFBIZ-2645

allow-html in service validation is too restrictive

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Won't Fix
    • Affects Version/s: Trunk
    • Fix Version/s: Trunk
    • Component/s: framework
    • Labels:
      None

      Description

      Service 'IN' parameters are validated. Default is allow-html='none'
      This filters out all the html chars. e.g one cannot set this text "Tom's age is likely > Paul's age"
      '>' is not allowed

      Rederers already escape html, so it may be best to keep validation alllow-html='any'. If service has a need to constrain, service should specify allow-html explicitly.

      Attaching patch. Please let me if this does not make sense.

      1. allow-html.diff
        4 kB
        Harmeet Bedi

        Activity

        Hide
        Harmeet Bedi added a comment -

        Ofbiz would not work if comments or description would be editable by a rich text editor. This is a severe restriction.. but it is a matter of policy. As secure as possible seems to be the policy.
        Feel free to close, but this does limit functionality. That seems to be tradeoff that project has taken.

        Show
        Harmeet Bedi added a comment - Ofbiz would not work if comments or description would be editable by a rich text editor. This is a severe restriction.. but it is a matter of policy. As secure as possible seems to be the policy. Feel free to close, but this does limit functionality. That seems to be tradeoff that project has taken.
        Hide
        Jacques Le Roux added a comment -

        Should we not close this issue?

        Show
        Jacques Le Roux added a comment - Should we not close this issue?
        Hide
        David E. Jones added a comment -

        OFBiz already has functionality to encode output in addition to the functionality that filters input. In discussions about this security aspect of the project we decided that doing both would be best, especially since there are possibilities of holes for both filtering input and encoding output.

        Please consider that we want the defaults to be as secure as possible and allow ways of doing things in a less restricted way when it is needed. If you find a field that needs to have HTML or special characters related to HTML then change the allow-html attribute to support that.

        In general I would say no, it would not be a good idea to relax the default security.

        Is there a specific place where you have run into this and would like to discuss a change for that, or are you mostly just looking at things generally?

        If we do want to consider changing this general policy we should have a discussion on the mailing list and see what people think. If there is a general consensus then we will go with that, and if not we can always vote on it. The best way to start such a discussion would be to write up a proposal and send it to the mailing list.

        Show
        David E. Jones added a comment - OFBiz already has functionality to encode output in addition to the functionality that filters input. In discussions about this security aspect of the project we decided that doing both would be best, especially since there are possibilities of holes for both filtering input and encoding output. Please consider that we want the defaults to be as secure as possible and allow ways of doing things in a less restricted way when it is needed. If you find a field that needs to have HTML or special characters related to HTML then change the allow-html attribute to support that. In general I would say no, it would not be a good idea to relax the default security. Is there a specific place where you have run into this and would like to discuss a change for that, or are you mostly just looking at things generally? If we do want to consider changing this general policy we should have a discussion on the mailing list and see what people think. If there is a general consensus then we will go with that, and if not we can always vote on it. The best way to start such a discussion would be to write up a proposal and send it to the mailing list.
        Hide
        Harmeet Bedi added a comment -

        Wondering if one should sanitize on the display side to address security issueWould similar to 'gettext' from dom may do that ?.
        i feel system should take the input the user desires to input and render what is safe.
        I think there is single point on the renderer side ModelFormField:getEntry where the security santization could be applied.
        This would also allows allows improvements on the rendering side to be made or new types of renderers added. e.g we(www.emforium.com) have a gwt based renderer that works well with html.(would like to contribute if i figure out how)

        allow-html='safe' seems nicer than 'none' from functionality pov.. but not sure how safe or how efficient. ofbiz has cms aspects.. input restrictions could impede making cms aspects richer. if 'safe' seems acceptable.. please do make the change and i will test it. personally prefer 'none' and santize on rendering side... can make patches for rendering side if you want me to do that.

        Show
        Harmeet Bedi added a comment - Wondering if one should sanitize on the display side to address security issueWould similar to 'gettext' from dom may do that ?. i feel system should take the input the user desires to input and render what is safe. I think there is single point on the renderer side ModelFormField:getEntry where the security santization could be applied. This would also allows allows improvements on the rendering side to be made or new types of renderers added. e.g we(www.emforium.com) have a gwt based renderer that works well with html.(would like to contribute if i figure out how) allow-html='safe' seems nicer than 'none' from functionality pov.. but not sure how safe or how efficient. ofbiz has cms aspects.. input restrictions could impede making cms aspects richer. if 'safe' seems acceptable.. please do make the change and i will test it. personally prefer 'none' and santize on rendering side... can make patches for rendering side if you want me to do that.
        Hide
        Adrian Crum added a comment -

        I think the idea was to default to high-security, or best-security. In other words, shut down everything, and only open up the exceptions.

        Show
        Adrian Crum added a comment - I think the idea was to default to high-security, or best-security. In other words, shut down everything, and only open up the exceptions.
        Hide
        Hans Bakker added a comment -

        Sorry this will allow html completely and is not acceptable. we could consider alllow-html='safe'. Any other opinions?

        Show
        Hans Bakker added a comment - Sorry this will allow html completely and is not acceptable. we could consider alllow-html='safe'. Any other opinions?
        Hide
        Harmeet Bedi added a comment -

        patch to make service default validation less restrictive

        Show
        Harmeet Bedi added a comment - patch to make service default validation less restrictive

          People

          • Assignee:
            Unassigned
            Reporter:
            Harmeet Bedi
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development