diaspora/diaspora

View on GitHub

Possible unprotected redirect
Open

        redirect_to params[:redirect_uri] + "?" + redirect_fragment

Unvalidated redirects and forwards are #10 on the OWASP Top Ten.

Redirects which rely on user-supplied values can be used to "spoof" websites or hide malicious links in otherwise harmless-looking URLs. They can also allow access to restricted areas of a site if the destination is not validated.

Brakeman will raise warnings whenever redirect_to appears to be used with a user-supplied value that may allow them to change the :host option.

For example,

redirect_to params.merge(:action => :home)

will create a warning like

Possible unprotected redirect near line 46: redirect_to(params)

This is because params could contain :host => 'evilsite.com' which would redirect away from your site and to a malicious site.

If the first argument to redirect_to is a hash, then adding :only_path => true will limit the redirect to the current host. Another option is to specify the host explicitly.

redirect_to params.merge(:only_path => true)

redirect_to params.merge(:host => 'myhost.com')

If the first argument is a string, then it is possible to parse the string and extract the path:

redirect_to URI.parse(some_url).path 

If the URL does not contain a protocol (e.g., http://), then you will probably get unexpected results, as redirect_to will prepend the current host name and a protocol.

Possible SQL injection
Open

    joins(comments_sql).joins(likes_sql)
Severity: Minor
Found in app/models/person.rb by brakeman

Injection is #1 on the 2013 OWASP Top Ten web security risks. SQL injection is when a user is able to manipulate a value which is used unsafely inside a SQL query. This can lead to data leaks, data loss, elevation of privilege, and other unpleasant outcomes.

Brakeman focuses on ActiveRecord methods dealing with building SQL statements.

A basic (Rails 2.x) example looks like this:

User.first(:conditions => "username = '#{params[:username]}'")

Brakeman would produce a warning like this:

Possible SQL injection near line 30: User.first(:conditions => ("username = '#{params[:username]}'")) 

The safe way to do this query is to use a parameterized query:

User.first(:conditions => ["username = ?", params[:username]])

Brakeman also understands the new Rails 3.x way of doing things (and local variables and concatenation):

username = params[:user][:name].downcase
password = params[:user][:password]

User.first.where("username = '" + username + "' AND password = '" + password + "'")

This results in this kind of warning:

Possible SQL injection near line 37:
User.first.where((((("username = '" + params[:user][:name].downcase) + "' AND password = '") + params[:user][:password]) + "'"))

See the Ruby Security Guide for more information and Rails-SQLi.org for many examples of SQL injection in Rails.

Possible SQL injection
Open

      .select(<<-SQL
Severity: Minor
Found in app/models/person.rb by brakeman

Injection is #1 on the 2013 OWASP Top Ten web security risks. SQL injection is when a user is able to manipulate a value which is used unsafely inside a SQL query. This can lead to data leaks, data loss, elevation of privilege, and other unpleasant outcomes.

Brakeman focuses on ActiveRecord methods dealing with building SQL statements.

A basic (Rails 2.x) example looks like this:

User.first(:conditions => "username = '#{params[:username]}'")

Brakeman would produce a warning like this:

Possible SQL injection near line 30: User.first(:conditions => ("username = '#{params[:username]}'")) 

The safe way to do this query is to use a parameterized query:

User.first(:conditions => ["username = ?", params[:username]])

Brakeman also understands the new Rails 3.x way of doing things (and local variables and concatenation):

username = params[:user][:name].downcase
password = params[:user][:password]

User.first.where("username = '" + username + "' AND password = '" + password + "'")

This results in this kind of warning:

Possible SQL injection near line 37:
User.first.where((((("username = '" + params[:user][:name].downcase) + "' AND password = '") + params[:user][:password]) + "'"))

See the Ruby Security Guide for more information and Rails-SQLi.org for many examples of SQL injection in Rails.

Unescaped model attribute
Open

      != t(".you_currently", count: current_user.invitation_code.count,
Severity: Minor
Found in app/views/admins/_user_search.haml by brakeman

Cross-site scripting (or XSS) is #3 on the 2013 [OWASP Top Ten](https://www.owasp.org/index.php/Top102013-A3-Cross-SiteScripting(XSS)) web security risks and it pops up nearly everywhere.

XSS occurs when a user-controlled value is displayed on a web page without properly escaping it, allowing someone to inject Javascript or HTML into the page which will be interpreted and executed by the browser..

In Rails 2.x, values need to be explicitly escaped (e.g., by using the h method). Since Rails 3.x, auto-escaping in views is enabled by default. However, one can still use the raw or html_safe methods to output a value directly.

See the Ruby Security Guide for more details.

Query Parameters and Cookies

ERB example:

<%= params[:query].html_safe %>

Brakeman looks for several situations that can allow XSS. The simplest is like the example above: a value from the params or cookies is being directly output to a view. In such cases, it will issue a warning like:

Unescaped parameter value near line 3: params[:query]

By default, Brakeman will also warn when a parameter or cookie value is used as an argument to a method, the result of which is output unescaped to a view.

For example:

<%= raw some_method(cookie[:name]) %>

This raises a warning like:

Unescaped cookie value near line 5: some_method(cookies[:oreo])

However, the confidence level for this warning will be weak, because it is not directly outputting the cookie value.

Some methods are known to Brakeman to either be dangerous (link_to is one) or safe (escape_once). Users can specify safe methods using the --safe-methods option. Alternatively, Brakeman can be set to only warn when values are used directly with the --report-direct option.

Model Attributes

Because (many) models come from database values, Brakeman mistrusts them by default.

For example, if @user is an instance of a model set in an action like

def set_user
  @user = User.first
end

and there is a view with

<%= @user.name.html_safe %>

Brakeman will raise a warning like

Unescaped model attribute near line 3: User.first.name

If you trust all your data (although you probably shouldn't), this can be disabled with --ignore-model-output.

Unsafe reflection method constantize called with model attribute
Open

        User.reflect_on_association(asso).class_name.constantize.where(id: ids).destroy_all
Severity: Minor
Found in lib/account_deleter.rb by brakeman

Brakeman reports on several cases of remote code execution, in which a user is able to control and execute code in ways unintended by application authors.

The obvious form of this is the use of eval with user input.

However, Brakeman also reports on dangerous uses of send, constantize, and other methods which allow creation of arbitrary objects or calling of arbitrary methods.

Unsafe reflection method constantize called with model attribute
Open

        Person.reflect_on_association(asso).class_name.constantize.where(id: ids).destroy_all
Severity: Minor
Found in lib/account_deleter.rb by brakeman

Brakeman reports on several cases of remote code execution, in which a user is able to control and execute code in ways unintended by application authors.

The obvious form of this is the use of eval with user input.

However, Brakeman also reports on dangerous uses of send, constantize, and other methods which allow creation of arbitrary objects or calling of arbitrary methods.

Possible SQL injection
Open

            "share_visibilities.shareable_type = '#{base_class}'")
Severity: Minor
Found in lib/diaspora/shareable.rb by brakeman

Injection is #1 on the 2013 OWASP Top Ten web security risks. SQL injection is when a user is able to manipulate a value which is used unsafely inside a SQL query. This can lead to data leaks, data loss, elevation of privilege, and other unpleasant outcomes.

Brakeman focuses on ActiveRecord methods dealing with building SQL statements.

A basic (Rails 2.x) example looks like this:

User.first(:conditions => "username = '#{params[:username]}'")

Brakeman would produce a warning like this:

Possible SQL injection near line 30: User.first(:conditions => ("username = '#{params[:username]}'")) 

The safe way to do this query is to use a parameterized query:

User.first(:conditions => ["username = ?", params[:username]])

Brakeman also understands the new Rails 3.x way of doing things (and local variables and concatenation):

username = params[:user][:name].downcase
password = params[:user][:password]

User.first.where("username = '" + username + "' AND password = '" + password + "'")

This results in this kind of warning:

Possible SQL injection near line 37:
User.first.where((((("username = '" + params[:user][:name].downcase) + "' AND password = '") + params[:user][:password]) + "'"))

See the Ruby Security Guide for more information and Rails-SQLi.org for many examples of SQL injection in Rails.

Possible SQL injection
Open

          " aspect_visibilities.shareable_type = '#{base_class}'")
Severity: Minor
Found in lib/diaspora/shareable.rb by brakeman

Injection is #1 on the 2013 OWASP Top Ten web security risks. SQL injection is when a user is able to manipulate a value which is used unsafely inside a SQL query. This can lead to data leaks, data loss, elevation of privilege, and other unpleasant outcomes.

Brakeman focuses on ActiveRecord methods dealing with building SQL statements.

A basic (Rails 2.x) example looks like this:

User.first(:conditions => "username = '#{params[:username]}'")

Brakeman would produce a warning like this:

Possible SQL injection near line 30: User.first(:conditions => ("username = '#{params[:username]}'")) 

The safe way to do this query is to use a parameterized query:

User.first(:conditions => ["username = ?", params[:username]])

Brakeman also understands the new Rails 3.x way of doing things (and local variables and concatenation):

username = params[:user][:name].downcase
password = params[:user][:password]

User.first.where("username = '" + username + "' AND password = '" + password + "'")

This results in this kind of warning:

Possible SQL injection near line 37:
User.first.where((((("username = '" + params[:user][:name].downcase) + "' AND password = '") + params[:user][:password]) + "'"))

See the Ruby Security Guide for more information and Rails-SQLi.org for many examples of SQL injection in Rails.

Possible SQL injection
Open

        where("#{table_name}.#{order} < ?", max_time).order("#{table_name}.#{order} DESC")
Severity: Minor
Found in lib/diaspora/shareable.rb by brakeman

Injection is #1 on the 2013 OWASP Top Ten web security risks. SQL injection is when a user is able to manipulate a value which is used unsafely inside a SQL query. This can lead to data leaks, data loss, elevation of privilege, and other unpleasant outcomes.

Brakeman focuses on ActiveRecord methods dealing with building SQL statements.

A basic (Rails 2.x) example looks like this:

User.first(:conditions => "username = '#{params[:username]}'")

Brakeman would produce a warning like this:

Possible SQL injection near line 30: User.first(:conditions => ("username = '#{params[:username]}'")) 

The safe way to do this query is to use a parameterized query:

User.first(:conditions => ["username = ?", params[:username]])

Brakeman also understands the new Rails 3.x way of doing things (and local variables and concatenation):

username = params[:user][:name].downcase
password = params[:user][:password]

User.first.where("username = '" + username + "' AND password = '" + password + "'")

This results in this kind of warning:

Possible SQL injection near line 37:
User.first.where((((("username = '" + params[:user][:name].downcase) + "' AND password = '") + params[:user][:password]) + "'"))

See the Ruby Security Guide for more information and Rails-SQLi.org for many examples of SQL injection in Rails.

Model attribute used in regex
Open

        .scan(/(?:^|\s)#([#{ActsAsTaggableOn::Tag.tag_text_regexp}]+|<3)/u)
Severity: Minor
Found in lib/diaspora/taggable.rb by brakeman

Denial of Service (DoS) is any attack which causes a service to become unavailable for legitimate clients.

For issues that Brakeman detects, this typically arises in the form of memory leaks.

Symbol DoS

Since Symbols are not garbage collected in Ruby versions prior to 2.2.0, creation of large numbers of Symbols could lead to a server running out of memory.

Brakeman checks for instances of user input which is converted to a Symbol. When this is not restricted, an attacker could create an unlimited number of Symbols.

The best approach is to simply never convert user-controlled input to a Symbol. If this cannot be avoided, use a whitelist of acceptable values.

For example:

valid_values = ["valid", "values", "here"]

if valid_values.include? params[:value]
  symbolized = params[:value].to_sym
end

Regex DoS

Regular expressions can be used for DoS if the pattern and input requires exponential time to process.

Brakeman will warn about dynamic regular expressions which may be controlled by an attacker. The attacker can create an "evil regex" and then supply input which causes the server to use a large amount of resources.

It is recommended to avoid interpolating user input into regular expressions.

Model attribute used in regex
Open

      regex =/(^|\s|>)#([#{ActsAsTaggableOn::Tag.tag_text_regexp}]+|&lt;3)/u
Severity: Minor
Found in lib/diaspora/taggable.rb by brakeman

Denial of Service (DoS) is any attack which causes a service to become unavailable for legitimate clients.

For issues that Brakeman detects, this typically arises in the form of memory leaks.

Symbol DoS

Since Symbols are not garbage collected in Ruby versions prior to 2.2.0, creation of large numbers of Symbols could lead to a server running out of memory.

Brakeman checks for instances of user input which is converted to a Symbol. When this is not restricted, an attacker could create an unlimited number of Symbols.

The best approach is to simply never convert user-controlled input to a Symbol. If this cannot be avoided, use a whitelist of acceptable values.

For example:

valid_values = ["valid", "values", "here"]

if valid_values.include? params[:value]
  symbolized = params[:value].to_sym
end

Regex DoS

Regular expressions can be used for DoS if the pattern and input requires exponential time to process.

Brakeman will warn about dynamic regular expressions which may be controlled by an attacker. The attacker can create an "evil regex" and then supply input which causes the server to use a large amount of resources.

It is recommended to avoid interpolating user input into regular expressions.

Model attribute used in regex
Open

      regex = /(?<=^|\s|>)#([#{ActsAsTaggableOn::Tag.tag_text_regexp}]+|<3)/u
Severity: Minor
Found in lib/diaspora/taggable.rb by brakeman

Denial of Service (DoS) is any attack which causes a service to become unavailable for legitimate clients.

For issues that Brakeman detects, this typically arises in the form of memory leaks.

Symbol DoS

Since Symbols are not garbage collected in Ruby versions prior to 2.2.0, creation of large numbers of Symbols could lead to a server running out of memory.

Brakeman checks for instances of user input which is converted to a Symbol. When this is not restricted, an attacker could create an unlimited number of Symbols.

The best approach is to simply never convert user-controlled input to a Symbol. If this cannot be avoided, use a whitelist of acceptable values.

For example:

valid_values = ["valid", "values", "here"]

if valid_values.include? params[:value]
  symbolized = params[:value].to_sym
end

Regex DoS

Regular expressions can be used for DoS if the pattern and input requires exponential time to process.

Brakeman will warn about dynamic regular expressions which may be controlled by an attacker. The attacker can create an "evil regex" and then supply input which causes the server to use a large amount of resources.

It is recommended to avoid interpolating user input into regular expressions.

The use of eval is a serious security risk.
Open

    eval(<<DATA