Cms: craft.request.getParam() XSS Protection

[mzlock]

Description

A developer can create a XSS vulnerability by adding a query parameter to their HTML.

Example:

{% set query = craft.request.getParam('q') %}

<h2>
    Search for: {{ query }}
</h2>

https://example.com/search?q=><SCRIPT>document.location=“https://www.google.com”<%2FSCRIPT>

Can craft.request.getParam() filter the query param and protect from these attacks?

Additional info

• Craft version: 2.6.2999
• PHP version: 7.0.8

[brandonkelly]

Twig actually protects you from this, by automatically HTML-encoding everything you output (unless you tell it not to using the|raw filter). So that template would just output this:

<h2>
    Search for: &gt;&lt;SCRIPT&gt;document.location=&quot;https://www.google.com&quot;&lt;/SCRIPT&gt;
</h2>

[codyjames]

For posterity, I want to note that if you are using the |t (or |translate) filter you will need to manually escape the query.

{{ "Search for: {query}"|t({ query: query|escape }) }}

[brandonkelly]

@codyjames the |t filter just returns a string, so it should still get auto-escaped by Twig without needing to add the |escape filter.

[codyjames]

@brandonkelly Thanks for info!

Figured out the issue I was having. We are using the Typogrify plugin:
{{ 'Search for: "{query}"'|t({ query: query })|typogrify }}
which is allowing the unescaped content. Not a good place to be using that without escaping the thing beforehand:
{{ 'Search for: "{query}"'|t({ query: query|escape })|typogrify }}

[khalwat]

@codyjames file an issue for me here? https://github.com/nystudio107/craft-typogrify/issues

I'll have a look to see if perhaps it shouldn't be returning a \Twig_Markup

Or at the very least, it should be documented that it returns it raw.

[khalwat]

@codyjames addressed in https://github.com/nystudio107/craft-typogrify/releases/tag/1.1.12

[khalwat]

So I actually had to end up reverting this:

https://github.com/nystudio107/craft-typogrify/releases/tag/1.1.13

Typogrify really does need to output raw HTML in order to operate. I added a node to the docs on security when dealing with untrusted user input:

https://github.com/nystudio107/craft-typogrify#security

[khalwat]

After being unsatisfied with just a warning, and some help from @brandonkelly ended up with -> https://github.com/nystudio107/craft-typogrify/releases/tag/1.1.14

Any untrusted strings it escapes before running its filters. Allowed me to do some nice refactoring too.