Nested scripts

[myxoh]

This related to the issue: flavorjones/loofah#127
When storing in a database it's more common not to store the characters encoded, making the nested script issue more of an issue than in the base library. I've already submitted a fix to Loofah that adds the new method recursive_scrub_fragment, I suggest xss_foliate use this scrub instead of the base scrub, I will start working on a PR as soon as the recursive scrubbing is merged into Loofah

[flavorjones]

Please see discussion on the referenced github issue.

[myxoh]

Hi. I'm sorry to be coming back one year later. I just saw a similar issue to the one reported on a server I've worked at.
After some examination, I think you were right not to modify further loofah. However, this is still an issue for loofah-activerecord

Here's the use case to make myself as clear as I can.

I have a Profile model that has a description. On this description I allow special characters for various things: For example This platform is use to describe math operands < > + / =, etc

We are using a rich text renderer that is susceptible to xss, but has too many nice features to avoid, so our compromise was using loofah-activerecord to at least remove obvious attempts at xss.

model Profile
  # some code....
  xss_foliate encode_special_chars: false
  # some more code...
end

This opens up to the vulnerability, originally described in flavorjones/loofah#127

When the user sets the description to:

<<s>script>alert('a')<<s>/script>

The output from loofah (with encode special chars false) is

<script>alert('a')</script>

Because our renderer is susceptible to XSS, this is a huge problem

I agree with you on the original topic that our use case is an 'edge caseforloofah`, which is why I didn't follow through before.

However, for the purposes of loofah-activerecord, the most useful feature is scrubbing, I can't choose to display anything BUT text, and I can't encode characters, else the renderer would display special characters.

Let me know if it is at all possible to sort this out on the loofah-active-record level. Else, we will try to come up with an alternative approach.

Thank you for your time.

[flavorjones]

@myxoh Thanks for bringing this up. I think you're still confusing text strings with HTML strings.

xss_foliate is intended to sanitize strings in your ActiveRecord models, that's it. Nothing else. If you want protection from XSS when you render your views, then you need to investigate methods to sanitize your views at render time.

As I explained at length in flavorjones/loofah#127, when you ask Loofah to leave < and > in your result by setting the option encode_special_chars: false, then you also need to take additional precautions before rendering that string into HTML.

If you 1) ask for > and < to remain unescaped, 2) take no additional steps to escape those characters, 3) render them unescaped into an HTML doc, then that is a vulnerability in your code and not in loofah or loofah-activerecord.

I strongly suggest that you simply look into using use Rails's html_escape (which is aliased to h in view code) or a variety of other escaping methods. This Stack Overflow article might be a good place to start, as it references quite a few different ways to html-escape strings.

I hope this response helps.