Struts 2
  1. Struts 2
  2. WW-3597

XSS vulnerability in javatemplates plugin

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2.1.1
    • Fix Version/s: 2.2.3
    • Labels:
      None
    • Flags:
      Important

      Description

      Many of the component handlers do not escape the value attribute. In fact they have been deliberately set to not escape their output. This enables reflective XSS on any page which uses the struts tags where the value is not manually escaped.

      The javatemplates plugin is increasingly being used instead of the default Freemarker renderer because of its performance benefits. The Freemarker renderer escapes values correclty therefore switching over to the javatemplates plugin can automatically make your website vulnerable.

      Also, the documentation should make it very clear which attributes are not encoded, for example, the anchor tag's href attribute is not encoded, therefore if you don't use the url tag to construct your url, then you need to make sure you escape any untrusted data you use to construct the url.

      I have updated all of the javatemplates plugins' tag handlers to be consistent with the Freemarker renderer and will attach a patch.

        Issue Links

          Activity

          Gareth Faires created issue -
          Gareth Faires made changes -
          Field Original Value New Value
          Attachment javatemplates-xss.patch [ 12474385 ]
          Hide
          Maurizio Cucchiara added a comment - - edited

          I'm pretty sure that default template engine escapes every value attributes, have you checked if this behavior adheres to the other template engines?

          Show
          Maurizio Cucchiara added a comment - - edited I'm pretty sure that default template engine escapes every value attributes, have you checked if this behavior adheres to the other template engines?
          Maurizio Cucchiara made changes -
          Assignee Maurizio Cucchiara [ maurizio.cucchiara ]
          Hide
          Gareth Faires added a comment - - edited

          If we take the textfield tag as an example, the default Freemarker engine, see text.ftl, constructs the value attribute as follows:
          <#if parameters.nameValue??>
          value="<@s.property value="parameters.nameValue"/>"<#rt/>
          </#if>

          This uses the property tag which, by default, escapes the output.

          Contrast this with the TextFieldHandler, which explicitly doesn't encode the value attribute.
          .addIfExists("value", params.get("nameValue"), false)

          To see this vulnerability in action, go to a form page with a url like:
          /SomeAction.action?anInputField="/><script>alert("XSS Flaw")</script>

          With the default Freemarker engine, the value is properly escaped, but with the javatemplates engine, you get the alert box.

          Show
          Gareth Faires added a comment - - edited If we take the textfield tag as an example, the default Freemarker engine, see text.ftl, constructs the value attribute as follows: <#if parameters.nameValue??> value="<@s.property value="parameters.nameValue"/>"<#rt/> </#if> This uses the property tag which, by default, escapes the output. Contrast this with the TextFieldHandler, which explicitly doesn't encode the value attribute. .addIfExists("value", params.get("nameValue"), false) To see this vulnerability in action, go to a form page with a url like: /SomeAction.action?anInputField="/><script>alert("XSS Flaw")</script> With the default Freemarker engine, the value is properly escaped, but with the javatemplates engine, you get the alert box.
          Hide
          Maurizio Cucchiara added a comment -

          Patch applied.
          Thanks Gareth

          Show
          Maurizio Cucchiara added a comment - Patch applied. Thanks Gareth
          Maurizio Cucchiara made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 2.2.2 [ 12315200 ]
          Resolution Fixed [ 1 ]
          Maurizio Cucchiara made changes -
          Link This issue duplicates WW-3608 [ WW-3608 ]
          Lukasz Lenart made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          20h 19m 1 Maurizio Cucchiara 24/Mar/11 08:13
          Resolved Resolved Closed Closed
          584d 13h 54m 1 Lukasz Lenart 28/Oct/12 22:08

            People

            • Assignee:
              Maurizio Cucchiara
              Reporter:
              Gareth Faires
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development