Coder Social home page Coder Social logo

prontolabs / pronto-brakeman Goto Github PK

View Code? Open in Web Editor NEW
18.0 5.0 36.0 101 KB

Pronto runner for Brakeman, security vulnerability scanner for RoR

License: MIT License

Ruby 100.00%
pronto pronto-runner brakeman ruby-on-rails security security-scanner analyzer

pronto-brakeman's People

Contributors

aergonaut avatar alessio-signorini avatar arlantir avatar ashkulz avatar bermannoah avatar doomspork avatar ermolaev avatar jeroenj avatar kevin-j-m avatar mehagar avatar mmozuras avatar petergoldstein avatar ridiculous avatar skateman avatar sunny avatar zenom avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar

pronto-brakeman's Issues

No warnings reported if Rails application is inside a subdirectory of the repo

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
[...]

Does not play well with pronto-rails_best_practices

When pronto-brakeman is included with pronto-rails_best_practices, pronto-rails_best_practicesis 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 is not configurable

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

No output

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.

nil (ArgumentError)

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.

Brakeman doesn't scan .erb 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.

running brakeman on a set of files only may miss warnings

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?

New version release

Any plans to release a new version?

fad1cb0

This feature would be really useful to us for ignoring false positives.

Update brakeman version - maybe be more flexible?

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!

Fails if repo doesn't contain a Rails application

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>'

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.