Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XSS False Positive #1081

Closed
AgoraSecurity opened this issue Aug 18, 2017 · 3 comments
Closed

XSS False Positive #1081

AgoraSecurity opened this issue Aug 18, 2017 · 3 comments

Comments

@AgoraSecurity
Copy link

I ran the scan against an app and I got the following result:

Medium | StrategiesController | show | Cross Site Scripting | Unsafe model attribute in link_to href near line 33: link_to(User.where(:id => Strategy.friendly.find..

The code is:

name = User.where(id: @strategy.userid).first.name
the_link = sanitize link_to name, link_url
@page_author = the_link.html_safe

I tried to exploit the XSS, but I couldn't.

I think the sanitize link_to name, link_url and the_link.html_safe make the code safe.

IMHO it's a false positive and the detection method of XSS can be improved. Anyhow if I'm wrong, please let me know.

@presidentbeef
Copy link
Owner

Hi,

Can you please run with either -f plain or -f json and provide the entire warning?

Additionally, please show where link_url comes from and where @page_author is being used (assuming that is the value on line 33).

The reason Brakeman warns on use of link_to is because URLs may start with javascript:. The call to sanitize may strip this.

Also, html_safe does not make a value safe, it makes it unsafe.

@AgoraSecurity
Copy link
Author

Thanks for the quick answer!

-f plain output:

Confidence: Medium
Category: Cross Site Scripting
Check: LinkToHref
Message: Unsafe model attribute in link_to href
Code: link_to(User.where(:id => Strategy.friendly.find(params[:id]).userid).first.name, ("/profile?uid=" + get_uid(Strategy.friendly.find(params[:id]).userid).to_s))
File: app/controllers/strategies_controller.rb
Line: 33

More context:
Vulnerable code:

    elsif can_view_category(params[:moment].id, params[:strategy].id, @category)
      link_url = '/profile?uid=' + get_uid(@category.userid).to_s
      name = User.where(id: @category.userid).first.name
      the_link = sanitize link_to name, link_url
      @page_author = the_link.html_safe
    else

File: app/views/layouts/application.html.erb

            <div class="align_right">
              <%= @page_author %>
            </div>

@presidentbeef
Copy link
Owner

Incidentally closed by #1093

Repository owner locked and limited conversation to collaborators Oct 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants