hpi-swt2/workshop-portal

View on GitHub

Showing 494 of 494 total issues

Potentially dangerous key allowed for mass assignment
Open

      user_params.permit(:email, :name, :password, :password_confirmation, :role, :current_password)

Mass assignment is a feature of Rails which allows an application to create a record from the values of a hash.

Example:

User.new(params[:user])

Unfortunately, if there is a user field called admin which controls administrator access, now any user can make themselves an administrator.

attr_accessible and attr_protected can be used to limit mass assignment. However, Brakeman will warn unless attr_accessible is used, or mass assignment is completely disabled.

There are two different mass assignment warnings which can arise. The first is when mass assignment actually occurs, such as the example above. This results in a warning like

Unprotected mass assignment near line 61: User.new(params[:user])

The other warning is raised whenever a model is found which does not use attr_accessible. This produces generic warnings like

Mass assignment is not restricted using attr_accessible

with a list of affected models.

In Rails 3.1 and newer, mass assignment can easily be disabled:

config.active_record.whitelist_attributes = true

Unfortunately, it can also easily be bypassed:

User.new(params[:user], :without_protection => true)

Brakeman will warn on uses of without_protection.

Avoid too many return statements within this method.
Open

      return 1
Severity: Major
Found in app/models/event.rb - About 30 mins to fix

    Avoid too many return statements within this method.
    Open

          return -1
    Severity: Major
    Found in app/models/event.rb - About 30 mins to fix

      Loofah 2.0.3 is vulnerable (CVE-2018-8048). Upgrade to 2.1.2
      Open

          loofah (2.0.3)
      Severity: Minor
      Found in Gemfile.lock by brakeman

      Avoid too many return statements within this method.
      Open

          return participant1.name <=> participant2.name
      Severity: Major
      Found in app/models/event.rb - About 30 mins to fix

        rails-html-sanitizer 1.0.3 is vulnerable (CVE-2018-3741). Upgrade to 1.0.4
        Open

            rails-html-sanitizer (1.0.3)
        Severity: Minor
        Found in Gemfile.lock by brakeman

        User controlled method execution
        Open

              @application_letters.sort_by! {|l| l.user.profile.send(params[:sort]) } if params[:sort]

        Using unfiltered user data to select a Class or Method to be dynamically sent is dangerous.

        It is much safer to whitelist the desired target or method.

        Unsafe use of method:

        method = params[:method]
        @result = User.send(method.to_sym)

        Safe:

        method = params[:method] == 1 ? :method_a : :method_b
        @result = User.send(method, *args)

        Unsafe use of target:

        table = params[:table]
        model = table.classify.constantize
        @result = model.send(:method)

        Safe:

        target = params[:target] == 1 ? Account : User
        @result = target.send(:method, *args)

        Including user data in the arguments passed to an Object#send is safe, as long as the method can properly handle potentially bad data.

        Safe:

        args = params["args"] || []
        @result = User.send(:method, *args)

        Potentially dangerous key allowed for mass assignment
        Open

              user_params.permit(:email, :name, :password, :password_confirmation, :role)

        Mass assignment is a feature of Rails which allows an application to create a record from the values of a hash.

        Example:

        User.new(params[:user])

        Unfortunately, if there is a user field called admin which controls administrator access, now any user can make themselves an administrator.

        attr_accessible and attr_protected can be used to limit mass assignment. However, Brakeman will warn unless attr_accessible is used, or mass assignment is completely disabled.

        There are two different mass assignment warnings which can arise. The first is when mass assignment actually occurs, such as the example above. This results in a warning like

        Unprotected mass assignment near line 61: User.new(params[:user])

        The other warning is raised whenever a model is found which does not use attr_accessible. This produces generic warnings like

        Mass assignment is not restricted using attr_accessible

        with a list of affected models.

        In Rails 3.1 and newer, mass assignment can easily be disabled:

        config.active_record.whitelist_attributes = true

        Unfortunately, it can also easily be bypassed:

        User.new(params[:user], :without_protection => true)

        Brakeman will warn on uses of without_protection.

        Method older_than_required_age_at_start_date_of_event? has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
        Open

          def older_than_required_age_at_start_date_of_event?(given_event, age)
            return false unless self.profile
            event_start = given_event.start_date
            event_start_is_before_birthday = event_start.month > self.profile.birth_date.month || (event_start.month == self.profile.birth_date.month && event_start.day >= self.profile.birth_date.day)
            age_at_event_start = event_start.year - self.profile.birth_date.year - (event_start_is_before_birthday ? 0 : 1)
        Severity: Minor
        Found in app/models/user.rb - About 25 mins to fix

        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

        Similar blocks of code found in 2 locations. Consider refactoring.
        Open

          def send_acceptance_emails
            event = Event.find(params[:id])
            @email = event.generate_acceptances_email
            @templates = [{subject: 'Zusage 1', content: 'Lorem Ispum...'}, {subject: 'Zusage 2', content: 'Lorem Ispum...'}, {subject: 'Zusage 3', content: 'Lorem Ispum...'}]
            render :email
        Severity: Minor
        Found in app/controllers/events_controller.rb and 1 other location - About 25 mins to fix
        app/controllers/events_controller.rb on lines 96..101

        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 30.

        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

        Further Reading

        Similar blocks of code found in 2 locations. Consider refactoring.
        Open

          def send_rejection_emails
            event = Event.find(params[:id])
            @email = event.generate_rejections_email
            @templates = [{subject: 'Absage 1', content: 'Lorem Ispum...'}, {subject: 'Absage 2', content: 'Lorem Ispum...'}, {subject: 'Absage 3', content: 'Lorem Ispum...'}]
            render :email
        Severity: Minor
        Found in app/controllers/events_controller.rb and 1 other location - About 25 mins to fix
        app/controllers/events_controller.rb on lines 88..93

        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 30.

        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

        Further Reading

        Rule doesn't have all its properties in alphabetical order.
        Open

        label.required:after {

        Inconsistent indentation detected.
        Open

            def too_big?(file)
              file.size > MAX_SIZE
            end
        Severity: Minor
        Found in app/models/agreement_letter.rb by rubocop

        This cops checks for inconsistent indentation.

        Example:

        class A
          def test
            puts 'hello'
             puts 'world'
          end
        end

        Inconsistent indentation detected.
        Open

            def wrong_filetype?(file)
              file.content_type != ALLOWED_MIMETYPE
            end
        Severity: Minor
        Found in app/models/agreement_letter.rb by rubocop

        This cops checks for inconsistent indentation.

        Example:

        class A
          def test
            puts 'hello'
             puts 'world'
          end
        end

        Line is too long. [100/80]
        Open

            accepted_applications = application_letters.where(status: ApplicationLetter.statuses[:accepted])
        Severity: Minor
        Found in app/models/event.rb by rubocop

        Favor modifier if usage when having a single-line body. Another good alternative is the usage of control flow &&/||.
        Open

            if participant2.requires_agreement_letter_for_event?(self)
        Severity: Minor
        Found in app/models/event.rb by rubocop

        Checks for if and unless statements that would fit on one line if written as a modifier if/unless. The maximum line length is configured in the Metrics/LineLength cop.

        Example:

        # bad
        if condition
          do_stuff(bar)
        end
        
        unless qux.empty?
          Foo.do_something
        end
        
        # good
        do_stuff(bar) if condition
        Foo.do_something unless qux.empty?

        Prefer single-quoted strings when you don't need string interpolation or special symbols.
        Open

          scope :draft_is, ->(draft) { where("draft = ?", draft) }
        Severity: Minor
        Found in app/models/event.rb by rubocop

        Checks if uses of quotes match the configured preference.

        Example: EnforcedStyle: single_quotes (default)

        # bad
        "No special symbols"
        "No string interpolation"
        "Just text"
        
        # good
        'No special symbols'
        'No string interpolation'
        'Just text'
        "Wait! What's #{this}!"

        Example: EnforcedStyle: double_quotes

        # bad
        'Just some text'
        'No special chars or interpolation'
        
        # good
        "Just some text"
        "No special chars or interpolation"
        "Every string in #{project} uses double_quotes"

        Use the new Ruby 1.9 hash syntax.
        Open

          validate :status_cannot_be_changed, :if => Proc.new { |letter| letter.status_changed?}
        Severity: Minor
        Found in app/models/application_letter.rb by rubocop

        This cop checks hash literal syntax.

        It can enforce either the use of the class hash rocket syntax or the use of the newer Ruby 1.9 syntax (when applicable).

        A separate offense is registered for each problematic pair.

        The supported styles are:

        • ruby19 - forces use of the 1.9 syntax (e.g. {a: 1}) when hashes have all symbols for keys
        • hash_rockets - forces use of hash rockets for all hashes
        • nomixedkeys - simply checks for hashes with mixed syntaxes
        • ruby19nomixed_keys - forces use of ruby 1.9 syntax and forbids mixed syntax hashes

        Example: EnforcedStyle: ruby19 (default)

        # bad
        {:a => 2}
        {b: 1, :c => 2}
        
        # good
        {a: 2, b: 1}
        {:c => 2, 'd' => 2} # acceptable since 'd' isn't a symbol
        {d: 1, 'e' => 2} # technically not forbidden

        Example: EnforcedStyle: hash_rockets

        # bad
        {a: 1, b: 2}
        {c: 1, 'd' => 5}
        
        # good
        {:a => 1, :b => 2}

        Example: EnforcedStyle: nomixedkeys

        # bad
        {:a => 1, b: 2}
        {c: 1, 'd' => 2}
        
        # good
        {:a => 1, :b => 2}
        {c: 1, d: 2}

        Example: EnforcedStyle: ruby19nomixed_keys

        # bad
        {:a => 1, :b => 2}
        {c: 2, 'd' => 3} # should just use hash rockets
        
        # good
        {a: 1, b: 2}
        {:c => 3, 'd' => 4}

        Line is too long. [83/80]
        Open

              can [:index, :show, :edit, :update, :destroy], Profile, user: { id: user.id }
        Severity: Minor
        Found in app/models/ability.rb by rubocop

        Use %i or %I for an array of symbols.
        Open

              can [:index, :show, :edit, :update, :destroy], ApplicationLetter, user: { id: user.id }
        Severity: Minor
        Found in app/models/ability.rb by rubocop

        This cop can check for array literals made up of symbols that are not using the %i() syntax.

        Alternatively, it checks for symbol arrays using the %i() syntax on projects which do not want to use that syntax.

        Configuration option: MinSize If set, arrays with fewer elements than this value will not trigger the cop. For example, a MinSize of3` will not enforce a style on an array of 2 or fewer elements.

        Example: EnforcedStyle: percent (default)

        # good
        %i[foo bar baz]
        
        # bad
        [:foo, :bar, :baz]

        Example: EnforcedStyle: brackets

        # good
        [:foo, :bar, :baz]
        
        # bad
        %i[foo bar baz]
        Severity
        Category
        Status
        Source
        Language