search

presidentbeef / brakeman

A static analysis security vulnerability scanner for Ruby on Rails applications
https://brakemanscanner.org/
Other
7.02k stars 732 forks source link

`eval` call not being detected #1850

Closed johansenja closed 4 months ago

johansenja commented 5 months ago

Background

Brakeman version: 6.1.2 Rails version: 7.0.8.4 Ruby version: 3.2.2

Link to Rails application code: https://github.com/johansenja/brakeman-erb-test

Issue

Evaluation check doesn't seem to be running. Judging by https://github.com/presidentbeef/brakeman/blob/main/lib/brakeman/checks/check_evaluation.rb, it seems like it should report any calls of eval - but in my sample application linked above it doesn't:

# https://github.com/johansenja/brakeman-erb-test/blob/master/app/models/application_record.rb
class ApplicationRecord < ActiveRecord::Base
  primary_abstract_class

  def foo(x)
    # is not picked up by brakeman
    eval x

    # is picked up by brakeman
    User.where("#{x}")
  end
end

Other Error

Run Brakeman with --debug to see the full stack trace.

Stack trace: ``` Loading scanner... Processing application in /Users/johansenja/code/brakeman-erb-test Processing gems... Parsing Gemfile [Notice] Detected Rails 7 application (Processing gems) Duration: 0.002076 seconds Processing configuration... Parsing config/environment.rb Parsing config/application.rb Parsing config/environments/production.rb [Notice] Escaping HTML by default (Processing configuration) Duration: 0.003562 seconds Parsing files... Parsing app/channels/application_cable/channel.rb Parsing app/helpers/application_helper.rb Parsing app/channels/application_cable/connection.rb Parsing app/controllers/application_controller.rb Parsing config/application.rb Parsing app/models/application_record.rb Parsing app/jobs/application_job.rb Parsing app/mailers/application_mailer.rb Parsing config/environment.rb Parsing config/boot.rb Parsing config/environments/development.rb Parsing config/environments/production.rb Parsing config/importmap.rb Parsing config/environments/test.rb Parsing config/initializers/assets.rb Parsing config/initializers/content_security_policy.rb Parsing config/initializers/filter_parameter_logging.rb Parsing config/initializers/inflections.rb Parsing config/initializers/permissions_policy.rb Parsing config/puma.rb Parsing config/routes.rb Parsing /Users/johansenja/code/brakeman-erb-test/app/views/comments_in_erb.html.erb Parsing app/views/comments_in_erb.html.erb app/views/comments_in_erb.html.erb:8 :: parse error on value "ok" (tIDENTIFIER) Could not parse app/views/comments_in_erb.html.erb Could not parse /Users/johansenja/code/brakeman-erb-test/app/views/comments_in_erb.html.erb Parsing /Users/johansenja/code/brakeman-erb-test/app/views/layouts/application.html.erb Parsing app/views/layouts/application.html.erb Parsing /Users/johansenja/code/brakeman-erb-test/app/views/layouts/mailer.html.erb Parsing app/views/layouts/mailer.html.erb (Parsing files) Duration: 0.103963 seconds Detecting file types... (Detecting file types) Duration: 0.000806 seconds Processing initializers... Processing /Users/johansenja/code/brakeman-erb-test/config/initializers/assets.rb (/Users/johansenja/code/brakeman-erb-test/config/initializers/assets.rb) Duration: 0.000107 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/initializers/filter_parameter_logging.rb (/Users/johansenja/code/brakeman-erb-test/config/initializers/filter_parameter_logging.rb) Duration: 5.5e-05 seconds (Processing initializers) Duration: 0.000189 seconds Processing libs... Processing /Users/johansenja/code/brakeman-erb-test/app/channels/application_cable/channel.rb (/Users/johansenja/code/brakeman-erb-test/app/channels/application_cable/channel.rb) Duration: 7.6e-05 seconds Processing /Users/johansenja/code/brakeman-erb-test/app/channels/application_cable/connection.rb (/Users/johansenja/code/brakeman-erb-test/app/channels/application_cable/connection.rb) Duration: 1.8e-05 seconds Processing /Users/johansenja/code/brakeman-erb-test/app/helpers/application_helper.rb (/Users/johansenja/code/brakeman-erb-test/app/helpers/application_helper.rb) Duration: 8.0e-06 seconds Processing /Users/johansenja/code/brakeman-erb-test/app/jobs/application_job.rb (/Users/johansenja/code/brakeman-erb-test/app/jobs/application_job.rb) Duration: 8.0e-06 seconds Processing /Users/johansenja/code/brakeman-erb-test/app/mailers/application_mailer.rb (/Users/johansenja/code/brakeman-erb-test/app/mailers/application_mailer.rb) Duration: 1.3e-05 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/boot.rb (/Users/johansenja/code/brakeman-erb-test/config/boot.rb) Duration: 2.0e-05 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/environment.rb (/Users/johansenja/code/brakeman-erb-test/config/environment.rb) Duration: 9.0e-06 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/environments/development.rb (/Users/johansenja/code/brakeman-erb-test/config/environments/development.rb) Duration: 0.000299 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/environments/test.rb (/Users/johansenja/code/brakeman-erb-test/config/environments/test.rb) Duration: 0.000135 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/importmap.rb (/Users/johansenja/code/brakeman-erb-test/config/importmap.rb) Duration: 2.4e-05 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/puma.rb (/Users/johansenja/code/brakeman-erb-test/config/puma.rb) Duration: 5.2e-05 seconds Processing /Users/johansenja/code/brakeman-erb-test/config/routes.rb (/Users/johansenja/code/brakeman-erb-test/config/routes.rb) Duration: 1.0e-05 seconds (Processing libs) Duration: 0.000738 seconds Processing routes... Parsing config/routes.rb (Processing routes) Duration: 0.000232 seconds Processing templates... Processing /Users/johansenja/code/brakeman-erb-test/app/views/layouts/application.html.erb (/Users/johansenja/code/brakeman-erb-test/app/views/layouts/application.html.erb) Duration: 0.000226 seconds Processing /Users/johansenja/code/brakeman-erb-test/app/views/layouts/mailer.html.erb (/Users/johansenja/code/brakeman-erb-test/app/views/layouts/mailer.html.erb) Duration: 4.3e-05 seconds (Processing templates) Duration: 0.000294 seconds Processing data flow in templates... Processing layouts/application (layouts/application) Duration: 0.000156 seconds Processing layouts/mailer (layouts/mailer) Duration: 3.7e-05 seconds (Processing data flow in templates) Duration: 0.000212 seconds Processing models... Processing /Users/johansenja/code/brakeman-erb-test/app/models/application_record.rb (/Users/johansenja/code/brakeman-erb-test/app/models/application_record.rb) Duration: 0.000122 seconds (Processing models) Duration: 0.000129 seconds Processing controllers... Processing /Users/johansenja/code/brakeman-erb-test/app/controllers/application_controller.rb (/Users/johansenja/code/brakeman-erb-test/app/controllers/application_controller.rb) Duration: 0.000203 seconds (Processing controllers) Duration: 0.000211 seconds Processing data flow in controllers... Processing ApplicationController (ApplicationController) Duration: 6.6e-05 seconds (Processing data flow in controllers) Duration: 7.8e-05 seconds Indexing call sites... (Indexing call sites) Duration: 0.000195 seconds Running checks in parallel... - CheckBasicAuth - CheckBasicAuthTimingAttack - CheckCrossSiteScripting - CheckContentTag - CheckCookieSerialization Automatic to_json escaping is enabled. Checking layouts/application for XSS Checking for XSS in content_tag - CheckCreateWith - CheckCSRFTokenForgeryCVE - CheckDefaultRoutes - CheckDeserialize - CheckDetailedExceptions - CheckDigestDoS - CheckDynamicFinders - CheckEOLRails - CheckEOLRuby - CheckEscapeFunction - CheckEvaluation - CheckExecute - CheckFileAccess - CheckFileDisclosure Finding possible file access Finding calls to load() Checking layouts/mailer for XSS Checking each controller for default routes Finding eval-like calls Processing eval-like calls Finding system calls using `` Finding other system calls Processing system calls Finding calls using FileUtils - CheckFilterSkipping - CheckForgerySetting - CheckHeaderDoS - CheckI18nXSS - CheckJRubyXML Processing found calls - CheckJSONEncoding - CheckJSONEntityEscape - CheckJSONParsing - CheckLinkTo - CheckLinkToHref - CheckMailTo - CheckMassAssignment - CheckMimeTypeDoS - CheckModelAttrAccessible - CheckModelAttributes - CheckModelSerialize - CheckNestedAttributes - CheckNestedAttributesBypass - CheckNumberToCurrency - CheckPageCachingCVE - CheckPathname - CheckPermitAttributes - CheckQuoteTableName - CheckRansack - CheckRedirect - CheckRegexDoS - CheckRender - CheckRenderDoS - CheckRenderInline Finding dynamic regexes Processing dynamic regexes Automatic to_json escaping is enabled. - CheckResponseSplitting - CheckRouteDoS - CheckSafeBufferManipulation - CheckSanitizeConfigCve - CheckSanitizeMethods - CheckSelectTag - CheckSelectVulnerability - CheckSend - CheckSendFile - CheckSessionManipulation Finding all calls to send_file() - CheckSessionSettings - CheckSimpleFormat - CheckSingleQuotes - CheckSkipBeforeFilter Finding instances of #send - CheckSprocketsPathTraversal - CheckSQL - CheckSQLCVEs - CheckSSLVerify - CheckStripTags - CheckSymbolDoSCVE - CheckTemplateInjection - CheckTranslateBug - CheckUnsafeReflection Finding calls to strip_tags() Finding ERB.new calls Processing ERB.new calls - CheckUnsafeReflectionMethods - CheckValidationRegex - CheckVerbConfusion - CheckWeakRSAKey - CheckWithoutProtection - CheckXMLDoS - CheckYAMLParsing Finding all mass assignments Processing all mass assignments Finding possible SQL calls on models Finding possible SQL calls with no target Finding possible SQL calls using constantized() Finding calls to named_scope or scope Processing possible SQL calls Checks finished, collecting results... Generating report... == Brakeman Report == Application Path: /Users/johansenja/code/brakeman-erb-test Rails Version: 7.0.8.4 Brakeman Version: 6.1.2 Scan Date: 2024-06-13 15:23:26 +0100 Duration: 0.119026 seconds Checks Run: BasicAuth, BasicAuthTimingAttack, CSRFTokenForgeryCVE, ContentTag, CookieSerialization, CreateWith, CrossSiteScripting, DefaultRoutes, Deserialize, DetailedExceptions, DigestDoS, DynamicFinders, EOLRails, EOLRuby, EscapeFunction, Evaluation, Execute, FileAccess, FileDisclosure, FilterSkipping, ForgerySetting, HeaderDoS, I18nXSS, JRubyXML, JSONEncoding, JSONEntityEscape, JSONParsing, LinkTo, LinkToHref, MailTo, MassAssignment, MimeTypeDoS, ModelAttrAccessible, ModelAttributes, ModelSerialize, NestedAttributes, NestedAttributesBypass, NumberToCurrency, PageCachingCVE, Pathname, PermitAttributes, QuoteTableName, Ransack, Redirect, RegexDoS, Render, RenderDoS, RenderInline, ResponseSplitting, RouteDoS, SQL, SQLCVEs, SSLVerify, SafeBufferManipulation, SanitizeConfigCve, SanitizeMethods, SelectTag, SelectVulnerability, Send, SendFile, SessionManipulation, SessionSettings, SimpleFormat, SingleQuotes, SkipBeforeFilter, SprocketsPathTraversal, StripTags, SymbolDoSCVE, TemplateInjection, TranslateBug, UnsafeReflection, UnsafeReflectionMethods, ValidationRegex, VerbConfusion, WeakRSAKey, WithoutProtection, XMLDoS, YAMLParsing == Overview == Controllers: 1 Models: 1 Templates: 2 Errors: 1 Security Warnings: 1 == Warning Types == SQL Injection: 1 == Controller Overview == Controller: ApplicationController Parent: ActionController::Base Routes: [None] == Template Output == layouts/application [Escaped Output] csrf_meta_tags [Escaped Output] csp_meta_tag [Escaped Output] stylesheet_link_tag("application", :"data-turbo-track" => "reload") [Escaped Output] javascript_importmap_tags [Escaped Output] yield layouts/mailer [Escaped Output] yield == Errors == Error: app/views/comments_in_erb.html.erb:8 :: parse error on value "ok" (tIDENTIFIER) Could not parse app/views/comments_in_erb.html.erb Location: Could not parse /Users/johansenja/code/brakeman-erb-test/app/views/comments_in_erb.html.erb == Warnings == Confidence: Weak Category: SQL Injection Check: SQL Message: Possible SQL injection Code: User.where("#{x}") File: app/models/application_record.rb Line: 9 ```

Ignore the errors about the erb parsing, it seems like that might be a separate issue which I reported here. More context: initially I wanted to test the erb parsing issue with a simple eval (a well-known insecure operation), to check brakeman would pick it up, but then I realised my eval call wasn't being reported even when the erb was parseable, or in a regular .rb file

presidentbeef commented 5 months ago

Hi @johansenja - Brakeman only warns about uses of eval that appear to evaluate user-provided values. Not every call to eval.

johansenja commented 4 months ago

Ah interesting, thanks - I hadn't realised there were conditions to it. It's curious that x is considered safe enough to be evaled but not safe enough to be interpolated in a sql statement