Possible unprotected redirect Open
redirect_to return_to
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
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 unprotected redirect Open
redirect_to params[:return_to] + hash_params
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
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.
Method handle_site_login_flow
has a Cognitive Complexity of 77 (exceeds 5 allowed). Consider refactoring. Open
def handle_site_login_flow
username = params[:user_session][:username] if params[:user_session]
u = User.find_by(username: username) || User.find_by(email: username)
if u && u.password_checker != 0
n = u.password_checker
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method handle_social_login_flow
has a Cognitive Complexity of 37 (exceeds 5 allowed). Consider refactoring. Open
def handle_social_login_flow(auth)
# Find an identity here
@identity = UserTag.find_with_omniauth(auth)
return_to = request.env['omniauth.origin'] || root_url
return_to += '?_=' + Time.now.to_i.to_s
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method subscribe_multiple_tag
has a Cognitive Complexity of 27 (exceeds 5 allowed). Consider refactoring. Open
def subscribe_multiple_tag(tag_list)
if !tag_list || tag_list == ''
flash[:notice] = "Please enter tags for subscription in the url."
else
if tag_list.is_a? String
- Read upRead up
- Create a ticketCreate a ticket
Cognitive Complexity
Cognitive Complexity is a measure of how difficult a unit of code is to intuitively understand. Unlike Cyclomatic Complexity, which determines how difficult your code will be to test, Cognitive Complexity tells you how difficult your code will be to read and comprehend.
A method's cognitive complexity is based on a few simple rules:
- Code is not considered more complex when it uses shorthand that the language provides for collapsing multiple statements into one
- Code is considered more complex for each "break in the linear flow of the code"
- Code is considered more complex when "flow breaking structures are nested"
Further reading
Method handle_site_login_flow
has 78 lines of code (exceeds 25 allowed). Consider refactoring. Open
def handle_site_login_flow
username = params[:user_session][:username] if params[:user_session]
u = User.find_by(username: username) || User.find_by(email: username)
if u && u.password_checker != 0
n = u.password_checker
- Create a ticketCreate a ticket
Method handle_social_login_flow
has 77 lines of code (exceeds 25 allowed). Consider refactoring. Open
def handle_social_login_flow(auth)
# Find an identity here
@identity = UserTag.find_with_omniauth(auth)
return_to = request.env['omniauth.origin'] || root_url
return_to += '?_=' + Time.now.to_i.to_s
- Create a ticketCreate a ticket
Method subscribe_multiple_tag
has 38 lines of code (exceeds 25 allowed). Consider refactoring. Open
def subscribe_multiple_tag(tag_list)
if !tag_list || tag_list == ''
flash[:notice] = "Please enter tags for subscription in the url."
else
if tag_list.is_a? String
- Create a ticketCreate a ticket
Avoid deeply nested control flow statements. Open
PasswordResetMailer.reset_notify(user, key).deliver_later unless user.nil? # respond the same to both successes and failures; security
- Create a ticketCreate a ticket
Avoid deeply nested control flow statements. Open
if session[:openid_return_to] # for openid login, redirects back to openid auth process
return_to = session[:openid_return_to]
session[:openid_return_to] = nil
redirect_to return_to + hash_params
else
- Create a ticketCreate a ticket
Avoid deeply nested control flow statements. Open
if current_user.crypted_password.nil? # the user has not created a pwd in the new site
flash[:warning] = I18n.t('user_sessions_controller.create_password_for_new_site')
redirect_to '/profile/edit'
else
flash[:notice] = I18n.t('user_sessions_controller.logged_in')
- Create a ticketCreate a ticket
Avoid deeply nested control flow statements. Open
if session[:openid_return_to] # for openid login, redirects back to openid auth process
return_to = session[:openid_return_to]
session[:openid_return_to] = nil
redirect_to return_to + hash_params
elsif params[:return_to] && params[:return_to].split('/')[0..3] == ["", "subscribe", "multiple", "tag"]
- Create a ticketCreate a ticket
Avoid deeply nested control flow statements. Open
unless params[:hash_params].to_s.empty?
hash_params = URI.parse("#" + params[:hash_params]).to_s
end
- Create a ticketCreate a ticket
Avoid deeply nested control flow statements. Open
params[:user_session][:openid_identifier] = 'https://old.publiclab.org/people/' + username + '/identity' if username
- Create a ticketCreate a ticket
Avoid more than 6 levels of block nesting. Open
if params[:return_to]
# params[:return_to] == /login?return_to=/subscribe/multiple/tag/tag1,tag299 ? true
return_to = '/' + params[:return_to].split('/')[2..-1].join('/') #== /subscribe/multiple/tag/tag1,tag299
redirect_to return_to
else
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks for excessive nesting of conditional and looping constructs.
You can configure if blocks are considered using the CountBlocks
option. When set to false
(the default) blocks are not counted
towards the nesting level. Set to true
to count blocks as well.
The maximum level of nesting allowed is configurable.
Similar blocks of code found in 2 locations. Consider refactoring. Open
def subscribe_multiple_tag(tag_list)
if !tag_list || tag_list == ''
flash[:notice] = "Please enter tags for subscription in the url."
else
if tag_list.is_a? String
- Read upRead up
- Create a ticketCreate a ticket
Duplicated Code
Duplicated code can lead to software that is hard to understand and difficult to change. The Don't Repeat Yourself (DRY) principle states:
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.
When you violate DRY, bugs and maintenance problems are sure to follow. Duplicated code has a tendency to both continue to replicate and also to diverge (leaving bugs as two similar implementations differ in subtle ways).
Tuning
This issue has a mass of 124.
We set useful threshold defaults for the languages we support but you may want to adjust these settings based on your project guidelines.
The threshold configuration represents the minimum mass a code block must have to be analyzed for duplication. The lower the threshold, the more fine-grained the comparison.
If the engine is too easily reporting duplication, try raising the threshold. If you suspect that the engine isn't catching enough duplication, try lowering the threshold. The best setting tends to differ from language to language.
See codeclimate-duplication
's documentation for more information about tuning the mass threshold in your .codeclimate.yml
.
Refactorings
- Extract Method
- Extract Class
- Form Template Method
- Introduce Null Object
- Pull Up Method
- Pull Up Field
- Substitute Algorithm
Further Reading
- Don't Repeat Yourself on the C2 Wiki
- Duplicated Code on SourceMaking
- Refactoring: Improving the Design of Existing Code by Martin Fowler. Duplicated Code, p76
Incorrect indentation detected (column 8 instead of 10). Open
# Successfully we have added subscription
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cops checks the indentation of comments.
Example:
# bad
# comment here
def method_name
end
# comment here
a = 'hello'
# yet another comment
if true
true
end
# good
# comment here
def method_name
end
# comment here
a = 'hello'
# yet another comment
if true
true
end
rescue
at 255, 12 is not aligned with end
at 259, 10. Open
rescue ActiveRecord::RecordInvalid
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop checks whether the rescue and ensure keywords are aligned properly.
Example:
# bad
begin
something
rescue
puts 'error'
end
# good
begin
something
rescue
puts 'error'
end
Use safe navigation (&.
) instead of checking if an object exists before calling the method. Open
elsif params[:return_to] && params[:return_to].split('/')[0..3] == ["", "subscribe", "multiple", "tag"]
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cop transforms usages of a method call safeguarded by a non nil
check for the variable whose method is being called to
safe navigation (&.
).
Configuration option: ConvertCodeThatCanStartToReturnNil
The default for this is false
. When configured to true
, this will
check for code in the format !foo.nil? && foo.bar
. As it is written,
the return of this code is limited to false
and whatever the return
of the method is. If this is converted to safe navigation,
foo&.bar
can start returning nil
as well as what the method
returns.
Example:
# bad
foo.bar if foo
foo.bar(param1, param2) if foo
foo.bar { |e| e.something } if foo
foo.bar(param) { |e| e.something } if foo
foo.bar if !foo.nil?
foo.bar unless !foo
foo.bar unless foo.nil?
foo && foo.bar
foo && foo.bar(param1, param2)
foo && foo.bar { |e| e.something }
foo && foo.bar(param) { |e| e.something }
# good
foo&.bar
foo&.bar(param1, param2)
foo&.bar { |e| e.something }
foo&.bar(param) { |e| e.something }
foo.nil? || foo.bar
!foo || foo.bar
# Methods that `nil` will `respond_to?` should not be converted to
# use safe navigation
foo.to_i if foo
Use 2 (not 0) spaces for indentation. Open
flash[:error] = tag.errors.full_messages
- Read upRead up
- Create a ticketCreate a ticket
- Exclude checks
This cops checks for indentation that doesn't use the specified number of spaces.
See also the IndentationConsistency cop which is the companion to this one.
Example:
# bad
class A
def test
puts 'hello'
end
end
# good
class A
def test
puts 'hello'
end
end
Example: IgnoredPatterns: ['^\s*module']
# bad
module A
class B
def test
puts 'hello'
end
end
end
# good
module A
class B
def test
puts 'hello'
end
end
end