Idea: Lower priority on XSS model attribute warnings if model validation is present

[oreoshake]

Bits and pieces of a convo:

<%= raw(model.thing) %> generates a high confidence warning due to the unescaped model attribute. However, this value passes through a few validations and the raw HTML does need to be displayed. What can brakeman do?

we know the model type... we can check the validations... but how do you determine that a regex/validation is html safe?

I guess you could register the regex/validation used as "this shit is safe"? You could lower the priority (to a minimum of medium) and add text indicating that the value is protected by validation.

I'm not entirely sure this is something to concern ourself with, just a though.

[presidentbeef]

We could do that.

[matt-glover]

I tend to think the use of raw suggests "I am intentionally rendering data directly so it is up to me to explicitly protect that data". While I might argue the warning should go away entirely when raw is called I know the brakeman standard is to prefer false positives to false negatives so I agree that reducing the confidence of the warning seems reasonable.

With that said the presence of a regex does not really tell us if the attribute actually passed the validation (perhaps it's a new record, data was stored before the validation was added, or it's a transient object with no db backing).

[oreoshake]

I tend to think the use of raw suggests "I am intentionally rendering data directly so it is up to me to explicitly protect that data". 

Good point. In rails 3, at least. At the same time, I've seen it used incorrectly more than once (one time it was MY code).

@claude thoughts?

[claude]

Yeah, from a development manager perspective it would be good to show the warning in case someone has used raw inappropriately.

On Jan 15, 2013, at 4:44 PM, Neil Matatall [email protected] wrote:

I tend to think the use of raw suggests "I am intentionally rendering data directly so it is up to me to explicitly protect that data". Good point. In rails 3, at least. At the same time, I've seen it used incorrectly more than once (one time it was MY code).

@claude thoughts?

— Reply to this email directly or view it on GitHub.

[jessieay]

I think this is the issue I am dealing with in https://github.com/presidentbeef/brakeman/issues/516#issuecomment-49242643

(do not want to raise for raw)

[presidentbeef]

This suggestion applies if you are using validators (e.g. validates_format_of) on the model attributes.

[jessieay]

Has anyone in this thread tried Rails' sanitize helper? From what I am reading, looks like this might be a way around using .html_safel or raw for user input while avoiding XSS vulnerability.

http://stackoverflow.com/questions/2985600/how-good-is-the-rails-sanitize-method http://api.rubyonrails.org/classes/ActionView/Helpers/SanitizeHelper.html#method-i-sanitize