Slide 1

Slide 1 text

Brakeman を欺く 2024.10.21 Kahiwa.rb #4 生活発表会 Koji NAKAMURA (@kozy4324)

Slide 2

Slide 2 text

Koji NAKAMURA ● 𝕏: @kozy4324 ● GitHub: @kozy4324 ● Classi株式会社所属 ● Kashiwa.rb主催 自己紹介

Slide 3

Slide 3 text

近況報告 この1ヶ月何してた? ● Kaigi on Rails 2024 の登壇準備 ● (それ以外の記憶があんまりないな...)

Slide 4

Slide 4 text

KoR パーカー も購入したよ 発送が間に合ってないけどな!

Slide 5

Slide 5 text

No content

Slide 6

Slide 6 text

No content

Slide 7

Slide 7 text

今日の話 ● Kaigi on Rails の登壇で Brakeman について喋ろうと思った内容は時間の都合で 大幅カットした ● 今日ここで供養したい

Slide 8

Slide 8 text

https://brakemanscanner.org/

Slide 9

Slide 9 text

Brakeman とは? ● Ruby on Rails Static Analysis Security Tool ● Rails 7.2 からはデフォルトで含まれる ○ rails new すると Gemfile に含まれる ● コードをスキャンして脆弱性疑いのある実装をレポートしてくれる ● SQL インジェクション脆弱性以外にも多くの脆弱性をチェックしてくれる ○ e.g.) XSS, コマンドインジェクション , オープンリダイレクト , etc…

Slide 10

Slide 10 text

Brakeman で SQL インジェクションを検知する class UsersController < ApplicationController def index @exists = User.exists?(params[:id]) end end app/controllers/users_controller.rb

Slide 11

Slide 11 text

== 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 の出力結果

Slide 12

Slide 12 text

本題: 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 インジェクションを検知するロジックをリバースエンジニアリング ● どういった実装をすると誤検知になるか(=欺けるか)を確認する

Slide 13

Slide 13 text

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

Slide 14

Slide 14 text

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

Slide 15

Slide 15 text

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 で起動する

Slide 16

Slide 16 text

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

Slide 17

Slide 17 text

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

Slide 18

Slide 18 text

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

Slide 19

Slide 19 text

ここまでのまとめ ● ruby_parser gem で AST(抽象構文木)を作ってそれをチェックしている ● 特定のクエリメソッド呼び出し有無をチェック ○ 特定のメソッドはレシーバを見ているが、基本的にレシーバ関係なく収集 ● クエリメソッドの呼び出し別に dangerous_value の有無をチェック ● dangerous_value がある場合 ○ dangerous_value にユーザーインプットが含まれていると confidence が High 、含まれていなけれ ば Medium ○ メソッドチェーンで、かつ先頭がターゲット(モデルクラス、 :connection、Arel)でない場合は confidence のレベルを一つ下げる ( High → Medium, Medium → weak )

Slide 20

Slide 20 text

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

Slide 21

Slide 21 text

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

Slide 22

Slide 22 text

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

Slide 23

Slide 23 text

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

Slide 24

Slide 24 text

だいたいわかった

Slide 25

Slide 25 text

期待する 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 の出力結果

Slide 26

Slide 26 text

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 になる

Slide 27

Slide 27 text

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 の出力結果

Slide 28

Slide 28 text

期待する 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 の出力結果

Slide 29

Slide 29 text

== 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 の出力結果

Slide 30

Slide 30 text

まとめ ● Brakeman の精度 ○ 怪しい実装はレポートするスタンスをとっている ○ 偽陽性が多く出る分に関しては仕方がないのではないだろうか ○ 検知が漏れるよりも100倍マシ(だと思う) ● 素朴に普通に Rails の実装をしよう ○ あまりにもトリッキーなことをすると Brakeman も脆弱性を検知できないぞ! ● AST(抽象構文木)の取り扱う実装を眺めてみて ○ ruby_parser gem の前提知識が足りないので深掘りしたくなった ○ ただし時代は Prism パーサーでは? ○ Brakeman も 6.2.1 で Prism サーバーに切り替えるオプションが入っている ○ Brakeman の今後の Prism パーサー対応も追いかけていこう

Slide 31

Slide 31 text

EOF