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

Brakeman raises warning in avo-dashboards gem #3563

Open
2 of 5 tasks
ObiWanKeoni opened this issue Dec 30, 2024 · 1 comment
Open
2 of 5 tasks

Brakeman raises warning in avo-dashboards gem #3563

ObiWanKeoni opened this issue Dec 30, 2024 · 1 comment

Comments

@ObiWanKeoni
Copy link
Contributor

Context

After adding brakeman for static security vulnerability analysis, I noticed the latest avo-dashboards version (3.15.7) raises a weak warning in 3 places:

== Warnings ==

Confidence: Weak
Category: Dynamic Render Path
Check: Render
Message: Render path contains parameter value
Code: render(action => Avo::TurboFrameWrapperComponent.new(params[:turbo_frame]), {})
File: ../usr/local/bundle/gems/avo-dashboards-3.15.7/app/views/avo/dashboards/cards/chartkick_missing.html.erb
Line: 1

Confidence: Weak
Category: Dynamic Render Path
Check: Render
Message: Render path contains parameter value
Code: render(action => Avo::TurboFrameWrapperComponent.new(params[:turbo_frame]), {})
File: ../usr/local/bundle/gems/avo-dashboards-3.15.7/app/views/avo/dashboards/cards/show.html.erb
Line: 1

Confidence: Weak
Category: Dynamic Render Path
Check: Render
Message: Render path contains parameter value
Code: render(action => Avo::PanelComponent.new(:name => Avo::Dashboards.dashboard_manager.get_dashboard_by_id(params[:id]).nam
e, :description => Avo::Dashboards.dashboard_manager.get_dashboard_by_id(params[:id]).description), {})
File: ../usr/local/bundle/gems/avo-dashboards-3.15.7/app/views/avo/dashboards/dashboards/show.html.erb
Line: 1

I think this can be remedied by using strong parameters:

def dashboard_id
  params.permit(:id)[:id]
end

# ...

render(action => Avo::PanelComponent.new(:name => Avo::Dashboards.dashboard_manager.get_dashboard_by_id(dashboard_id).nam
e, :description => Avo::Dashboards.dashboard_manager.get_dashboard_by_id(dashboard_id).description)

System configuration

Avo version: 3.15.7

Rails version: 8.0.1

Ruby version: 3.3.4

License type

  • Community
  • Pro
  • Advanced

Are you using Avo monkey patches, overriding views or view components?

  • Yes. If so, please post code samples.
  • No

Screenshots or screen recordings

N/A

@zhephyn
Copy link

zhephyn commented Jan 5, 2025

hey @Paul-Bob and @ObiWanKeoni . I've worked on the above brakeman warnings and solved them in my code base locally. Wanted to submit a PR to fix. However, when i run "brakeman" in the terminal, i noticed other warnings flagged by brakeman in addition to the ones you highlighted. Should i solve these too before submitting the PR or its okay to submit a PR for a fix restricted to only the warnings you highlighted above? Below is a snippet of my terminal output to show the additional warnings:
`== Warnings ==

Confidence: High
Category: Redirect
Check: Redirect
Message: Possible unprotected redirect
Code: redirect_to((params[:referrer] or resource_view_response_path))
File: app/controllers/avo/associations_controller.rb
Line: 281

Confidence: Medium
Category: Mass Assignment
Check: MassAssignment
Message: Specify exact keys allowed for mass assignment instead of using permit! which allows any keys
Code: params.permit!
File: app/helpers/avo/application_helper.rb
Line: 110

Confidence: Medium
Category: Mass Assignment
Check: MassAssignment
Message: Specify exact keys allowed for mass assignment instead of using permit! which allows any keys
Code: params.permit!
File: app/views/avo/partials/_table_header.html.erb
Line: 69

Confidence: Medium
Category: Mass Assignment
Check: MassAssignment
Message: Specify exact keys allowed for mass assignment instead of using permit! which allows any keys
Code: params.permit!
File: app/views/avo/partials/_view_toggle_button.html.erb
Line: 30

Confidence: Medium
Category: SQL Injection
Check: SQL
Message: Possible SQL injection
Code: summary_query.group(params[:field_id].to_sym)
File: app/controllers/avo/charts_controller.rb
Line: 6
`
As for the avo version i'm running:

zhephyn@zhephyn-HP-Notebook-PC:~/Desktop/github_projects/avo$ rails runner 'puts Avo::VERSION' warning: parser/current is loading parser/ruby33, which recognizes 3.3.6-compliant syntax, but you are running 3.3.1. Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri. 3.15.7

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

No branches or pull requests

2 participants