False negative XSS

[presidentbeef]

False negative XSS

[presidentbeef]

This being caught depends on what @search is and how it is set.

[oreoshake]

rails 2? 3? using rails_xss?

[philcox]

Using Rails 235, no rails_xss. Looking at https://github.com/nzkoz/rails_xss

the params hash gets passed into @search it is initialized from the following code:

class SearchForm < OpenStruct

attr_reader :params

def initialize(data, param_name, *args)
data ||= {}
contents = {}
@params = {}

args.each do |item|
  contents[item]                    = data[item]
  @params["#{param_name}[#{item}]"] = data[item]
end

super(contents)

end

end

[presidentbeef]

Do you have an example of it being set in a controller?

This is only going to be caught if it's something like

def all
  @search = SearchForm.new(params, ...)
end

[philcox]

Why will it only catch if called in a controller?

[presidentbeef]

Brakeman doesn't do whole program analysis (at least not at the moment, probably never since the Rails stack is huge), it looks at the regular Rails data flow from controllers to views, basically like a web request would do.

[philcox]

so this is the controller action:
def index
params[:category] ||= default_category
@search = search_form(:s, :params => [ :category, :filter_type, :filter_value, :order, :advanced ])

and 'search_form' is a controller method defined as:
def search_form(param_name, opts = {})
SearchForm.new(params[param_name], param_name, *opts[:params])
end

[presidentbeef]

Okay. Brakeman is not going to look any further than @search = search_form(:s, :params => [ :category, :filter_type, :filter_value, :order, :advanced ]) (i.e., not into the body of search_form). That's why this is not raising a warning.

It's a limitation of Brakeman at the moment (it doesn't really do inter-procedural analysis), but I'm thinking about supporting "simple" helper methods like this.

[philcox]

Got it. Thanks.