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

False negative SQLi using map #1651

Open
tsigouris007 opened this issue Nov 23, 2021 · 2 comments
Open

False negative SQLi using map #1651

tsigouris007 opened this issue Nov 23, 2021 · 2 comments

Comments

@tsigouris007
Copy link

Background

Brakeman version: 5.1.2
Rails version: 6.1.4.1
Ruby version: 2.5.5

Rails application code

Cannot disclose the full code but I can disclose the offending part which seems pretty clear.

    if params[:search_text].present?
      if params[:exact_match].presence == 'true'
        @quotes = @quotes.where(reply: params[:search_text])
      else
        search_query = params[:search_text].split(' ').
          map { |word| "CONCAT(' ', reply, ' ') LIKE '% #{word} %'" }.
          join(' AND ')
        @quotes = @quotes.where(search_query)
      end
    end

Issue

False negative SQLi using .map.
This is actually a legit SQL Injection point but brakeman fails to catch it.

@presidentbeef
Copy link
Owner

Haven't looked at this in-depth yet, but it's possible Brakeman doesn't know what @quotes is and that's why it's failing to catch it.

@tsigouris007
Copy link
Author

Hello @presidentbeef ,
quotes are defined earlier as:

class Earth::QuotesPresenter
  attr_reader :quotes

  def fetch
    @quotes = Quote.all

    # SOME OTHER CODE

    if params[:search_text].present?
      if params[:exact_match].presence == 'true'
        @quotes = @quotes.where(reply: params[:search_text])
      else
        search_query = params[:search_text].split(' ').
          map { |word| "CONCAT(' ', reply, ' ') LIKE '% #{word} %'" }.
          join(' AND ')
        @quotes = @quotes.where(search_query)
      end
    end
  end

Then you hit the URL using search_text=SQLI (avoiding spaces because they are split) and exact_match=false.
Brakeman is being run on the whole project and not only on this file so I guess the @quotes variable is being resolved. Could it not be resolved by brakeman somehow?
Because in my rails console when I query for pry(main)> Quote.all it properly returns all quotes from the database and when running the project the results are presented properly inside the page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants