Lock in $30 Savings on PRO—Offer Ends Soon! ⏳

Brakeman を欺く - Kashiwa.rb #4

Brakeman を欺く - Kashiwa.rb #4

Koji NAKAMURA

October 21, 2024
Tweet

More Decks by Koji NAKAMURA

Other Decks in Technology

Transcript

  1. Brakeman とは? • Ruby on Rails Static Analysis Security Tool

    • Rails 7.2 からはデフォルトで含まれる ◦ rails new すると Gemfile に含まれる • コードをスキャンして脆弱性疑いのある実装をレポートしてくれる • SQL インジェクション脆弱性以外にも多くの脆弱性をチェックしてくれる ◦ e.g.) XSS, コマンドインジェクション , オープンリダイレクト , etc…
  2. Brakeman で SQL インジェクションを検知する class UsersController < ApplicationController def index

    @exists = User.exists?(params[:id]) end end app/controllers/users_controller.rb
  3. == Brakeman Report == Application Path: /Users/kozy4324/work/my-rails-app Rails Version: 7.1.4

    Brakeman Version: 6.2.1 Scan Date: 2024-10-20 11:18:41 +0900 Duration: 0.497845 seconds Checks Run: SQL == Overview == Controllers: 2 Models: 3 Templates: 3 Errors: 0 Security Warnings: 1 == Warning Types == SQL Injection: 1 == Warnings == Confidence: High Category: SQL Injection Check: SQL Message: Possible SQL injection Code: User.exists?(params[:id]) File: app/controllers/users_controller.rb Line: 3 $ brakeman -t SQL の出力結果
  4. 本題: Brakeman を欺く? • ドキュメントより ◦ > Only the developers

    of an application can understand if certain values are dangerous or not. By default, Brakeman is extremely suspicious. This can lead to many “false positives.” • SQL インジェクションを検知するロジックをリバースエンジニアリング • どういった実装をすると誤検知になるか(=欺けるか)を確認する
  5. Brakeman のチェックする実装を確認する (1) $ rdbg -e "b Brakeman::FileParser#parse_ruby" -e "open

    vscode" -c -- brakeman --no-threads -t SQL brakeman を rdbg で起動する def parse_ruby input, path if path.is_a? Brakeman ::FilePath path = path.relative end Brakeman .debug "Parsing #{ path}" if @use_prism begin parse_with_prism input, path rescue => e Brakeman .debug "Prism failed to parse #{ path}: #{e}" parse_with_ruby_parser input, path end else parse_with_ruby_parser input, path end end https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/file_parser.rb#L82
  6. Brakeman のチェックする実装を確認する (2) def parse_with_ruby_parser input, path begin RubyParser .new.parse

    input, path, @timeout rescue Racc::ParseError => e raise e.exception(e.message + "\nCould not parse #{ path}") rescue Timeout::Error => e raise Exception .new("Parsing #{ path} took too long (> #{ @timeout } seconds). Try increasing the limit with --parser-timeout" ) rescue => e raise e.exception(e.message + "\nWhile processing #{ path}") end end https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/file_parser.rb#L108
  7. def run_check # Avoid reporting `user_input` on silly values when

    generating warning. # Note that we retroactively find `user_input` inside the "dangerous" value. @safe_input_attributes .merge IGNORE_METHODS_IN_SQL @sql_targets = [:average, :calculate , :count, :count_by_sql , :delete_all , :destroy_all , :find_by_sql , :maximum, :minimum, :pluck, :sum, :update_all ] @sql_targets .concat [:from, :group, :having, :joins, :lock, :order, :reorder, :where] if tracker.options[:rails3] @sql_targets .concat [:find_by, :find_by!, :find_or_create_by , :find_or_create_by! , :find_or_initialize_by , :not] if tracker.options[:rails4] if tracker.options[:rails6] @sql_targets .concat [:delete_by , :destroy_by , :rewhere, :reselect] @sql_targets .delete :delete_all @sql_targets .delete :destroy_all end if version_between?( "6.1.0", "9.9.9") @sql_targets .delete :order @sql_targets .delete :reorder @sql_targets .delete :pluck end Brakeman のチェックする実装を確認する (3) https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L21 $ rdbg -e "b Brakeman::CheckSQL#run_check" -e "open vscode" -c -- brakeman --no-threads -t SQL 改めて brakeman を rdbg で起動する
  8. Brakeman のチェックする実装を確認する (4) @expected_targets = active_record_models. keys + [:connection, :"ActiveRecord::Base"

    , :Arel] Brakeman.debug "Finding possible SQL calls on models" calls = tracker.find_call(:methods => @sql_targets , :nested => true) narrow_targets = [:exists?, :select] calls.concat tracker.find_call(:targets => active_record_models. keys, :methods => narrow_targets, :chained => true) Brakeman.debug "Finding possible SQL calls with no target" calls.concat tracker.find_call(:target => nil, :methods => @sql_targets ) Brakeman.debug "Finding possible SQL calls using constantized()" calls.concat tracker.find_call(:methods => @sql_targets ).select { |result| constantize_call? result } https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L56
  9. dangerous_value = case method when :find check_find_arguments call. second_arg when

    :exists? check_exists call. first_arg when :delete_all , :destroy_all check_find_arguments call. first_arg when :named_scope , :scope check_scope_arguments call when :find_by_sql , :count_by_sql check_by_sql_arguments call. first_arg when :calculate check_find_arguments call. third_arg when :last, :first, :all check_find_arguments call. first_arg when :average , :count, :maximum , :minimum , :sum if call.length > 5 unsafe_sql?(call. first_arg) or check_find_arguments(call. last_arg) else check_find_arguments call. last_arg end when :where, :rewhere , :having, :find_by , :find_by! , :find_or_create_by , :find_or_create_by! , :find_or_initialize_by ,:not, :delete_by , :destroy_by check_query_arguments call. arglist Brakeman のチェックする実装を確認する (5) https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L179
  10. if dangerous_value add_result result input = include_user_input? dangerous_value if input

    confidence = :high user_input = input else confidence = :medium user_input = dangerous_value end if result[ :call].target and result[ :chain] and not @expected_targets .include? result[ :chain].first confidence = case confidence when :high :medium when :medium :weak else confidence end end Brakeman のチェックする実装を確認する (6) https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L222
  11. ここまでのまとめ • ruby_parser gem で AST(抽象構文木)を作ってそれをチェックしている • 特定のクエリメソッド呼び出し有無をチェック ◦ 特定のメソッドはレシーバを見ているが、基本的にレシーバ関係なく収集

    • クエリメソッドの呼び出し別に dangerous_value の有無をチェック • dangerous_value がある場合 ◦ dangerous_value にユーザーインプットが含まれていると confidence が High 、含まれていなけれ ば Medium ◦ メソッドチェーンで、かつ先頭がターゲット(モデルクラス、 :connection、Arel)でない場合は confidence のレベルを一つ下げる ( High → Medium, Medium → weak )
  12. Brakeman のチェックする実装を確認する (7) #Checks the given expression for unsafe SQL

    values. If an unsafe value is #found, returns that value (may be the given _exp_ or a subexpression). # #Otherwise, returns false/nil. def unsafe_sql? exp, ignore_hash = false return unless sexp?(exp) dangerous_value = find_dangerous_value exp, ignore_hash safe_value?(dangerous_value) ? false : dangerous_value end https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L447
  13. Brakeman のチェックする実装を確認する (8) #Check _exp_ for dangerous values. Used by

    unsafe_sql? def find_dangerous_value exp, ignore_hash case exp.node_type when :lit, :str, :const, :colon2, :true, :false, :nil nil : (中略) : when :call unless IGNORE_METHODS_IN_SQL .include? exp.method if has_immediate_user_input? exp exp elsif TO_STRING_METHODS .include? exp.method find_dangerous_value exp. target, ignore_hash else check_call exp end end : end https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L458
  14. Brakeman のチェックする実装を確認する (9) def safe_value? exp return true unless sexp?

    exp case exp.node_type when :str, :lit, :const, :colon2, :nil, :true, :false true when :call if exp.method == :to_s or exp.method == :to_sym safe_value? exp. target else ignore_call? exp end when :if safe_value? exp. then_clause and safe_value? exp. else_clause when :block, :rlist safe_value? exp. last when :or safe_value? exp. lhs and safe_value? exp. rhs when :dstr not unsafe_string_interp? exp else false end end https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L601
  15. Brakeman のチェックする実装を確認する (10) def ignore_call? exp return unless call? exp

    ignore_methods_in_sql. include? exp.method or quote_call? exp or arel? exp or exp. method.to_s.end_with? "_id" or number_target? exp or date_target? exp or locale_call? exp end https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L626 IGNORE_METHODS_IN_SQL = Set[:id, :merge_conditions , :table_name , :quoted_table_name , :quoted_primary_key , :to_i, :to_f, :sanitize_sql , :sanitize_sql_array , :sanitize_sql_for_assignment , :sanitize_sql_for_conditions , :sanitize_sql_hash , :sanitize_sql_hash_for_assignment , :sanitize_sql_hash_for_conditions , :to_sql, :sanitize , :primary_key , :table_name_prefix , :table_name_suffix , :where_values_hash , :foreign_key , :uuid, :escape, :escape_string ] https://github.com/presidentbeef/brakeman/blob/v6.2.2/lib/brakeman/checks/check_sql.rb#L589
  16. 期待する Brakeman の出力(脆弱性の問題がある) class UsersController < ApplicationController def index @users

    = User.where("name = '#{params[:cond]}'") end end app/controllers/users_controller.rb == Warnings == Confidence: High Category: SQL Injection Check: SQL Message: Possible SQL injection Code: User.where("name = '#{params[:cond]}'") File: app/controllers/users_controller.rb Line: 3 $ brakeman -t SQL の出力結果
  17. Brakeman を欺く - 式展開する値をメソッド経由にする class UsersController < ApplicationController def index

    @users = User.where("name = '#{cond}'") end def cond params[:cond] end end app/controllers/users_controller.rb == Warnings == Confidence: Medium Category: SQL Injection Check: SQL Message: Possible SQL injection Code: User.where("name = '#{cond}'") File: app/controllers/users_controller.rb Line: 3 $ brakeman -t SQL の出力結果 Confidence が Medium になる
  18. Brakeman を欺く - 引数をメソッド経由にする class UsersController < ApplicationController def index

    @users = User.where(sql) end def sql "name = '#{params[:cond]}'" end end app/controllers/users_controller.rb == Warnings == No warnings found $ brakeman -t SQL の出力結果
  19. 期待する Brakeman の出力(脆弱性の問題はない) class UsersController < ApplicationController def index @val

    = params["sort_by_#{params[:attr]}"] end end app/controllers/users_controller.rb == Warnings == No warnings found $ brakeman -t SQL の出力結果
  20. == Warnings == Category: SQL Injection Check: SQL Message: Possible

    SQL injection Code: find_by("sort_by_#{params[:attr]}") File: app/controllers/users_controller.rb Line: 3 Brakeman を欺く - クエリメソッドと I/F を被らせる class UsersController < ApplicationController def index @val = find_by "sort_by_#{params[:attr]}" end def find_by key params[key] end end app/controllers/users_controller.rb $ brakeman -t SQL の出力結果
  21. まとめ • Brakeman の精度 ◦ 怪しい実装はレポートするスタンスをとっている ◦ 偽陽性が多く出る分に関しては仕方がないのではないだろうか ◦ 検知が漏れるよりも100倍マシ(だと思う)

    • 素朴に普通に Rails の実装をしよう ◦ あまりにもトリッキーなことをすると Brakeman も脆弱性を検知できないぞ! • AST(抽象構文木)の取り扱う実装を眺めてみて ◦ ruby_parser gem の前提知識が足りないので深掘りしたくなった ◦ ただし時代は Prism パーサーでは? ◦ Brakeman も 6.2.1 で Prism サーバーに切り替えるオプションが入っている ◦ Brakeman の今後の Prism パーサー対応も追いかけていこう
  22. EOF