prontolabs / pronto-brakeman Goto Github PK
View Code? Open in Web Editor NEWPronto runner for Brakeman, security vulnerability scanner for RoR
License: MIT License
Pronto runner for Brakeman, security vulnerability scanner for RoR
License: MIT License
I have a monorepo for which I am running pronto
with brakeman
runner. The Rails application is inside apps/backend
subdirectory. It does not report any warnings (even though it should and manual invocation reports some warnings), probably this is caused by the fact that Brakeman is always executed passing repo_path
, instead of the path that was passed to pronto.
Running pronto
with brakeman
:
$ pronto run -r=brakeman -c develop apps/backend/
To use retry middleware with Faraday v2.0+, install `faraday-retry` gem
Running Pronto::Brakeman
Running brakeman
manually NOT passing a path (so it defaults to repo_path
):
$ brakeman
Loading scanner...
Please supply the path to a Rails application (looking in /home/jakub/dev/ynd/nao/bb).
Use `--force` to run a scan anyway.
Running brakeman
manually passing a path to a subdirectory containing a Rails app:
$ brakeman apps/backend/
Loading scanner...
Processing application in /home/jakub/dev/someapp/apps/backend
[...]
Confidence: High
Category: File Access
Check: FileAccess
Message: Parameter value used in file name
Code: File.open(params[:id])
File: app/controllers/email_verifications_controller.rb
Line: 19
[...]
When pronto-brakeman
is included with pronto-rails_best_practices
, pronto-rails_best_practices
is unable to find any violations it finds when pronto-brakeman
isn't present. Interestingly pronto-brakeman
does not appear to bother any other pronto runners, at least not the ones I'm using. I confirmed it was pronto-brakeman
that was causing the issue with pronto-rails_best_practices
.
Here is my setup
# Gemfile
group :development do
gem 'pronto', '~> 0.7', require: false # Runs analysis by checking only the relevant changes
gem 'pronto-brakeman', '~> 0.7', require: false # Runs brakeman through pronto
gem 'pronto-fasterer', '~> 0.7', require: false # Runs fasterer through pronto
gem 'pronto-flay', '~> 0.7', require: false # Runs flay through pronto
gem 'pronto-rails_best_practices', '~> 0.7', require: false # Runs rails_best_practices through pronto
gem 'pronto-reek', '~> 0.7', require: false # Runs reek through pronto
gem 'pronto-rubocop', '~> 0.7', require: false # Runs rubocop through pronto
end
# .pronto.yml
all:
exclude:
- 'config/**/*'
- 'vendor/**/*'
- 'spec/**/*'
max_warnings: 150
github:
slug: owner/repo
With both pronto-brakeman
and pronto-rails_best_practices
with my comments on which runner is creating each violation
$ pronto run -c origin/develop
app/controllers/pronto_controller.rb:8 F: Possible security vuln... # pronto-brakeman
app/proto/test_wrapper.rb:14 W: Using each_with_index is slower... # pronto-fasterer
app/controllers/pronto_controller.rb:12 W: Similar code found in ... # pronto-flay
app/controllers/pronto_controller.rb:1 I: Has no descriptive comm... # pronto-reek
app/proto/test_wrapper.rb:8 I: Use `&&` instead of `and`. # pronto-rubocop
Now without pronto-brakeman
in Gemfile
$ pronto run -c origin/develop
app/proto/test_wrapper.rb:14 W: Using each_with_index is slower... # pronto-fasterer
app/controllers/pronto_controller.rb:12 W: Similar code found in ... # pronto-flay
app/controllers/pronto_controller.rb:2 W: Remove unused methods... # pronto-rails_best_practices
app/controllers/pronto_controller.rb:1 I: Has no descriptive comm... # pronto-reek
app/proto/test_wrapper.rb:8 I: Use `&&` instead of `and`. # pronto-rubocop
Interestingly, when I ran the pronto
gem locally reversing the order of the runners execution with the following change:
# lib/pronto/runners.rb
module Pronto
class Runners
def initialize(runners = Runner.runners, config = Config.new)
# @runners = runners -- Original Line
@runners = runners.reverse! # My new line
@config = config
end
end
end
I was able to get both pronto-brakeman and pronto-rails_best_practices to create violations, as shown below.
$ pronto run -c origin/develop
app/proto/test_wrapper.rb:8 I: Use `&&` instead of `and`. # pronto-rubocop
app/controllers/pronto_controller.rb:1 I: Has no descriptive comm... # pronto-reek
app/controllers/pronto_controller.rb:2 W: Remove unused methods... # pronto-rails_best_practices
app/controllers/pronto_controller.rb:12 W: Similar code found in ... # pronto-flay
app/proto/test_wrapper.rb:14 W: Using each_with_index is slower... # pronto-fasterer
app/controllers/pronto_controller.rb:8 F: Possible security vuln... # pronto-brakeman
So it appears that there is something that pronto-brakeman
is doing that doesn't affect the other runners, but does affect pronto-rails_best_practices
for some reason. I tried figuring out a fix, but there doesn't appear to be anything noticeably different between the other runners and either pronto-brakeman
or #pronto-rails_best_practices
.
See referenced issues on pronto-rails_best_practices
and pronto
as well
prontolabs/pronto-rails_best_practices#4
prontolabs/pronto#180
Brakeman has a number of options we can pass it, but currently this gem doesn't support using the pronto config file to do so. (One thing that would be nice is to be able to pass the -A
flag, so that users can run all of the checks Brakeman supports.)
I've created a new Rails project, I made an initial commit, and I've installed pronto
and pronto-brakeman
:
# ...
group :development do
# ...
gem 'pronto'
gem 'pronto-brakeman', require: false
end
# ...
And then: $ bundle
.
I've deleted the end
keyword in app/controllers/application_controller.rb
without commiting.
When I execute brakeman
I can see the following output:
Loading scanner...
Processing application in testing-app
Processing gems...
[Notice] Detected Rails 5 application
Processing configuration...
[Notice] Escaping HTML by default
Parsing files...
Processing initializers...
Processing libs...ed
Processing routes...
Processing templates...
Processing data flow in templates...
Processing models...
Processing controllers...
Processing data flow in controllers...
Indexing call sites...
Running checks in parallel...
- CheckBasicAuth
- CheckBasicAuthTimingAttack
- CheckCrossSiteScripting
- CheckContentTag
- CheckCreateWith
- CheckDefaultRoutes
- CheckDeserialize
- CheckDetailedExceptions
- CheckDigestDoS
- CheckDynamicFinders
- CheckEscapeFunction
- CheckEvaluation
- CheckExecute
- CheckFileAccess
- CheckFileDisclosure
- CheckFilterSkipping
- CheckForgerySetting
- CheckHeaderDoS
- CheckI18nXSS
- CheckJRubyXML
- CheckJSONEncoding
- CheckJSONParsing
- CheckLinkTo
- CheckLinkToHref
- CheckMailTo
- CheckMassAssignment
- CheckMimeTypeDoS
- CheckModelAttrAccessible
- CheckModelAttributes
- CheckModelSerialize
- CheckNestedAttributes
- CheckNestedAttributesBypass
- CheckNumberToCurrency
- CheckPermitAttributes
- CheckQuoteTableName
- CheckRedirect
- CheckRegexDoS
- CheckRender
- CheckRenderDoS
- CheckRenderInline
- CheckResponseSplitting
- CheckRouteDoS
- CheckSafeBufferManipulation
- CheckSanitizeMethods
- CheckSelectTag
- CheckSelectVulnerability
- CheckSend
- CheckSendFile
- CheckSessionManipulation
- CheckSessionSettings
- CheckSimpleFormat
- CheckSingleQuotes
- CheckSkipBeforeFilter
- CheckSQL
- CheckSQLCVEs
- CheckSSLVerify
- CheckStripTags
- CheckSymbolDoSCVE
- CheckTranslateBug
- CheckUnsafeReflection
- CheckValidationRegex
- CheckWithoutProtection
- CheckXMLDoS
- CheckYAMLParsing
Checks finished, collecting results...
Generating report...
== Brakeman Report ==
Application Path: testing-app
Rails Version: 5.2.2
Brakeman Version: 4.3.1
Scan Date: 2018-12-13 07:15:29 -0200
Duration: 0.051262497 seconds
Checks Run: BasicAuth, BasicAuthTimingAttack, ContentTag, CreateWith, CrossSiteScripting, DefaultRoutes, Deserialize, DetailedExceptions, DigestDoS, DynamicFinders, EscapeFunction, Evaluation, Execute, FileAccess, FileDisclosure, FilterSkipping, ForgerySetting, HeaderDoS, I18nXSS, JRubyXML, JSONEncoding, JSONParsing, LinkTo, LinkToHref, MailTo, MassAssignment, MimeTypeDoS, ModelAttrAccessible, ModelAttributes, ModelSerialize, NestedAttributes, NestedAttributesBypass, NumberToCurrency, PermitAttributes, QuoteTableName, Redirect, RegexDoS, Render, RenderDoS, RenderInline, ResponseSplitting, RouteDoS, SQL, SQLCVEs, SSLVerify, SafeBufferManipulation, SanitizeMethods, SelectTag, SelectVulnerability, Send, SendFile, SessionManipulation, SessionSettings, SimpleFormat, SingleQuotes, SkipBeforeFilter, StripTags, SymbolDoSCVE, TranslateBug, UnsafeReflection, ValidationRegex, WithoutProtection, XMLDoS, YAMLParsing
== Overview ==
Controllers: 0
Models: 1
Templates: 2
Errors: 1
Security Warnings: 0
== Warning Types ==
== Errors ==
Error: testing-app/app/controllers/application_controller.rb:2 :: parse error on value "$end" ($end)
Location: Could not parse testing-app/app/controllers/application_controller.rb
No warnings found
Executing pronto run
, pronto run --unstaged
and pronto run --staged
does not give any output at all.
build failed on rails 6 rc1.
logs
$ if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then bundle exec pronto run -f github_pr -c origin/${TRAVIS_BRANCH}; fi
/home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/sexp_processor-4.12.1/lib/sexp.rb:214:in `line': setting s(:args).line nil (ArgumentError)
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:43:in `deep_clone'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:32:in `block in deep_clone'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:30:in `each'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:30:in `deep_clone'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:32:in `block in deep_clone'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:30:in `each'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:30:in `deep_clone'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:32:in `block in deep_clone'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:30:in `each'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/ruby_parser/bm_sexp.rb:30:in `deep_clone'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/processors/alias_processor.rb:49:in `process_safely'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/processor.rb:92:in `process_initializer'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/scanner.rb:194:in `process_initializer'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/scanner.rb:188:in `block in process_initializers'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/scanner.rb:311:in `block in track_progress'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/scanner.rb:308:in `each'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/scanner.rb:308:in `track_progress'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/scanner.rb:186:in `process_initializers'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman/scanner.rb:47:in `process'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman.rb:361:in `scan'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/brakeman-4.5.1/lib/brakeman.rb:80:in `run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-brakeman-0.10.0/lib/pronto/brakeman.rb:13:in `run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/lib/pronto/runners.rb:20:in `block in run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/lib/pronto/runners.rb:13:in `each'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/lib/pronto/runners.rb:13:in `run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/lib/pronto.rb:64:in `run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/lib/pronto/cli.rb:66:in `block in run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/lib/pronto/cli.rb:64:in `chdir'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/lib/pronto/cli.rb:64:in `run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/command.rb:27:in `run'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in `invoke_command'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor.rb:387:in `dispatch'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/thor-0.20.3/lib/thor/base.rb:466:in `start'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/gems/pronto-0.10.0/bin/pronto:6:in `<top (required)>'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/bin/pronto:23:in `load'
from /home/travis/build/reckerswartz/tramitexpress_web/vendor/bundle/ruby/2.6.0/bin/pronto:23:in `<main>'
from /home/travis/.rvm/gems/ruby-2.6.3/bin/ruby_executable_hooks:24:in `eval'
from /home/travis/.rvm/gems/ruby-2.6.3/bin/ruby_executable_hooks:24:in `<main>'
Running Pronto::Brakeman
The command "if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then bundle exec pronto run -f github_pr -c origin/${TRAVIS_BRANCH}; fi" exited with 1.
presidentbeef/brakeman#1267 (comment)
Would it be possible to move to using skip-files
here
something like
files = ruby_patches.map do |patch|
patch.new_file_full_path.relative_path_from(repo_path).to_s
end
unchanged_files = Dir["**/*.rb"] - files
output = ::Brakeman.run(app_path: repo_path,
output_formats: [:to_s],
skip_files: unchanged_files)
At the moment it looks as though this gem only runs brakeman on ruby files, not .erb files. Because these are an important part of a Rails app's threat surface, it would be nice to include them in the corpus of patches that this gem scans.
I've seen examples of where brakeman can miss mass assignment warnings in rails 3 when using the only_files option. I have a fork currently that runs against the entire project, then allows messages_for to filter out the warnings not applicable to the change set that seems to work. Would the project be interested in the contribution?
Hey, I was wondering if I am missing something. I would like to run brakeman with a higher minimum confidence level, otherwise the noise is too high.
I saw that the ::Brakeman.run method has this as an option (https://github.com/presidentbeef/brakeman/blob/6af53c63feb909d19bab970aedb3b0c583073eb6/lib/brakeman.rb#LL54C18-L54C18)
would it make sense to have that configurability in the same manner as the run_all_checks
option?
Any plans to release a new version?
This feature would be really useful to us for ignoring false positives.
Could you please rev the version number and publish an updated Gem. The old gem requires an old brakeman, 2.4.3. https://rubygems.org/gems/pronto-brakeman
In the meantime, fetching master solves this:
gem 'pronto-brakeman', github: 'mmozuras/pronto-brakeman', ref: '64a733e'
I covered this in presidentbeef/brakeman#551
The currently-specified version of brakeman is obsolete.
I think in this case, pronto would be better served by using ">=" instead of "~>" when specifying the version of Brakeman. That'd be more flexible, and would avoid this problem in the future.
Thanks!
Running pronto with pronto-brakeman installed on a non-Rails project (in this case, Reek) results in an error:
/home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/brakeman-3.0.3/lib/brakeman/scanner.rb:28:in `initialize': Please supply the path to a Rails application. (Brakeman::NoApplication)
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/brakeman-3.0.3/lib/brakeman.rb:320:in `new'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/brakeman-3.0.3/lib/brakeman.rb:320:in `scan'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/brakeman-3.0.3/lib/brakeman.rb:62:in `run'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/pronto-brakeman-0.4.1/lib/pronto/brakeman.rb:15:in `run'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/pronto-0.4.1/lib/pronto.rb:60:in `block in run_all_runners'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/pronto-0.4.1/lib/pronto.rb:59:in `map'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/pronto-0.4.1/lib/pronto.rb:59:in `run_all_runners'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/pronto-0.4.1/lib/pronto.rb:34:in `run'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/pronto-0.4.1/lib/pronto/cli.rb:52:in `run'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/thor-0.19.1/lib/thor/command.rb:27:in `run'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/thor-0.19.1/lib/thor/invocation.rb:126:in `invoke_command'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/thor-0.19.1/lib/thor.rb:359:in `dispatch'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/thor-0.19.1/lib/thor/base.rb:440:in `start'
from /home/matijs/.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/pronto-0.4.1/bin/pronto:6:in `<top (required)>'
from /home/matijs/.rbenv/versions/2.2.2/bin/pronto:23:in `load'
from /home/matijs/.rbenv/versions/2.2.2/bin/pronto:23:in `<main>'
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.