Grails
  1. Grails
  2. GRAILS-9906

Encoding/escaping/XSS prevention improvements

    Details

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

      Description

      This is a super issue incorporating changes to encoding and escaping features to help developers write safe web applications. It provides better XSS prevention support.

      Some of the ideas are on this wiki page in Github:
      https://github.com/grails/grails-core/wiki/Default-Codecs

      Previous mailing list threads:
      http://grails.1312388.n4.nabble.com/Grails-2-0-Change-default-codec-to-html-td3709223.html#a3709340

        Issue Links

          Activity

          Hide
          Lari Hotari added a comment -

          Implementation has been starting in scb-encoding-support branch.

          It now contains changes to Grails StreamCharBuffer to support tracking the encoding state. StreamCharBuffer is the output buffer used in GSPs / taglibs. This solves the "double encoding problem" since we now have the information in the buffer of which "block" of the buffer has already been encoded.

          Show
          Lari Hotari added a comment - Implementation has been starting in scb-encoding-support branch . It now contains changes to Grails StreamCharBuffer to support tracking the encoding state. StreamCharBuffer is the output buffer used in GSPs / taglibs. This solves the "double encoding problem" since we now have the information in the buffer of which "block" of the buffer has already been encoded.
          Hide
          Lari Hotari added a comment -
          Show
          Lari Hotari added a comment - Diff of master and scb-encoding-support branch: https://github.com/grails/grails-core/compare/master...scb-encoding-support
          Show
          Lari Hotari added a comment - Rails 3 escaping is fairly simple: https://github.com/rails/rails/blob/3-2-stable/activesupport/lib/active_support/core_ext/string/output_safety.rb
          Show
          Lari Hotari added a comment - link to grails-dev email thread: http://grails.1312388.n4.nabble.com/Grails-2-3-encoding-escaping-xss-prevention-improvements-tt4642167.html
          Hide
          Lari Hotari added a comment -
          Show
          Lari Hotari added a comment - Blog post by Marc Palmer: http://grailsrocks.com/blog/2013/4/19/can-i-pwn-your-grails-application
          Hide
          Lari Hotari added a comment -

          I've pushed some changes to scb-safebuffer branch: https://github.com/grails/grails-core/commits/scb-safebuffer . This branch has the "safe buffer" solution I've had as a goal.

          There are a few things on my todo list that should be done before 2.3 M1 release:

          • rename "mimeType" to "contentType" in the code, for consistency
          • add separate active "codec" setting for taglibs internally (taglibCodec)
          • solve how inheriting active codecs should behave. Do we need "inherit to next" or "inherit to all" separately?
          • implement the configuration of default codecs
          • implement the configuration of default codecs for plugins
          • implement codec aliasing to solve the html -> xml or html4 "redirection"
          • check how the "safebuffer" should check if a buffer part should be applied. is the current "isSafe" solution ok? Do we need a separate solution for checking if a certain codec should be applied or not
          • for example: q=$ {params.q.encodeAsURL().encodeAsJavaScript()}

            might not work currently. Explicit encoding should not use "isSafe" for checking if a codec should be applied?

          implement unit tests for these usecases:

          • output should be safe at the end
          • detailed test document showing a GSP + Taglib that uses a default HTML codec but also writes out JS data inline in the GSP, and writes out JS data inline using a call to a TagLib, and a call to a tag that renders pre-escaped HTML content, and so on
          • tag output must not be automatically encoded.
          • tag call as function call should use defaultEncodeAs / encodeAsForTags settings
          • scriptlets should apply outCodec
          • double encoding should be prevented
          • Plugins cannot have their pages break because the app developer changes default codec setting.
          • Ideally the user should never need to explicitly think about codecs or calling them except in rare situations.
          • Add a function/tag to switch the current default codec - effectively pushing and popping a default codec stack. This could take the form of a withCodec(name, Closure) method in tags.
          • Use this function/tag in core tags like <g:javascript> and <r:script> to automatically set an appropriate codec
          • <g:render> and similar tags would need to set default codec to HTML again when including another GSP, pushing whatever was default onto a stack
          • Add support for an optional encodeAs attribute to all tags automatically, such that the result will be encoded with that codec if specified i.e. var s = \$ {g.createLink(...., encodeAs:'JavaScript')}
          • All GSPs in app or plugins default to HTML codec unless developer does something to change that using directive/tag
          • All outputs of expressions/inline code apply the current default codec
          • Tags are responsible for the correct encoding of their output, unless specified in encodeAs= attribute
          • It's possible to use raw codec to mark some output as something that shouldn't be escaped
          • support map argument to encodeAs attribute so that templateCodec, pageCodec & defaultCode can be changed separately
          Show
          Lari Hotari added a comment - I've pushed some changes to scb-safebuffer branch: https://github.com/grails/grails-core/commits/scb-safebuffer . This branch has the "safe buffer" solution I've had as a goal. There are a few things on my todo list that should be done before 2.3 M1 release: rename "mimeType" to "contentType" in the code, for consistency add separate active "codec" setting for taglibs internally (taglibCodec) solve how inheriting active codecs should behave. Do we need "inherit to next" or "inherit to all" separately? implement the configuration of default codecs implement the configuration of default codecs for plugins implement codec aliasing to solve the html -> xml or html4 "redirection" check how the "safebuffer" should check if a buffer part should be applied. is the current "isSafe" solution ok? Do we need a separate solution for checking if a certain codec should be applied or not for example: q=$ {params.q.encodeAsURL().encodeAsJavaScript()} might not work currently. Explicit encoding should not use "isSafe" for checking if a codec should be applied? implement unit tests for these usecases: output should be safe at the end detailed test document showing a GSP + Taglib that uses a default HTML codec but also writes out JS data inline in the GSP, and writes out JS data inline using a call to a TagLib, and a call to a tag that renders pre-escaped HTML content, and so on tag output must not be automatically encoded. tag call as function call should use defaultEncodeAs / encodeAsForTags settings scriptlets should apply outCodec double encoding should be prevented Plugins cannot have their pages break because the app developer changes default codec setting. Ideally the user should never need to explicitly think about codecs or calling them except in rare situations. Add a function/tag to switch the current default codec - effectively pushing and popping a default codec stack. This could take the form of a withCodec(name, Closure) method in tags. Use this function/tag in core tags like <g:javascript> and <r:script> to automatically set an appropriate codec <g:render> and similar tags would need to set default codec to HTML again when including another GSP, pushing whatever was default onto a stack Add support for an optional encodeAs attribute to all tags automatically, such that the result will be encoded with that codec if specified i.e. var s = \$ {g.createLink(...., encodeAs:'JavaScript')} All GSPs in app or plugins default to HTML codec unless developer does something to change that using directive/tag All outputs of expressions/inline code apply the current default codec Tags are responsible for the correct encoding of their output, unless specified in encodeAs= attribute It's possible to use raw codec to mark some output as something that shouldn't be escaped support map argument to encodeAs attribute so that templateCodec, pageCodec & defaultCode can be changed separately
          Hide
          Graeme Rocher added a comment -

          Lari - can this be closed now?

          Show
          Graeme Rocher added a comment - Lari - can this be closed now?
          Hide
          Lari Hotari added a comment -

          FYI "CVE-2013-6430 POSSIBLE XSS WHEN USING SPRING MVC" in Spring's JavaScriptUtils.javaScriptEscape https://jira.springsource.org/browse/SPR-9983
          JavaScriptUtils.javaScriptEscape is not used in Grails 2.3.x . It is used in 2.2.x (and before): https://github.com/grails/grails-core/blob/2.2.x/grails-plugin-codecs/src/main/groovy/org/codehaus/groovy/grails/plugins/codecs/JavaScriptCodec.groovy#L28

          Show
          Lari Hotari added a comment - FYI "CVE-2013-6430 POSSIBLE XSS WHEN USING SPRING MVC" in Spring's JavaScriptUtils.javaScriptEscape https://jira.springsource.org/browse/SPR-9983 JavaScriptUtils.javaScriptEscape is not used in Grails 2.3.x . It is used in 2.2.x (and before): https://github.com/grails/grails-core/blob/2.2.x/grails-plugin-codecs/src/main/groovy/org/codehaus/groovy/grails/plugins/codecs/JavaScriptCodec.groovy#L28

            People

            • Assignee:
              Lari Hotari
              Reporter:
              Lari Hotari
            • Votes:
              3 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development