Skip to content

Commit

Permalink
Merge pull request #1902 from presidentbeef/update_deserialize_check
Browse files Browse the repository at this point in the history
Always warn about deserializing with Marshal
  • Loading branch information
presidentbeef authored Dec 30, 2024
2 parents d834150 + f891743 commit 5f37891
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
5 changes: 4 additions & 1 deletion lib/brakeman/checks/check_deserialize.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,13 @@ def check_deserialize result, target, arg = nil
confidence = :high
elsif input = include_user_input?(arg)
confidence = :medium
elsif target == :Marshal
confidence = :low
message = msg("Use of ", msg_code("#{target}.#{method}"), " may be dangerous")
end

if confidence
message = msg(msg_code("#{target}.#{method}"), " called with ", msg_input(input))
message ||= msg(msg_code("#{target}.#{method}"), " called with ", msg_input(input))

warn :result => result,
:warning_type => "Remote Code Execution",
Expand Down
4 changes: 4 additions & 0 deletions test/apps/rails8/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
class ApplicationController < ActionController::Base
# Only allow modern browsers supporting webp images, web push, badges, import maps, CSS nesting, and CSS :has.
allow_browser versions: :modern

def deserialize_it
Marshal.load(something) # Always warns
end
end
16 changes: 15 additions & 1 deletion test/tests/rails8.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def expected
controller: 0,
model: 0,
template: 0,
warning: 2
warning: 3
}
end

Expand Down Expand Up @@ -47,4 +47,18 @@ def test_dangerous_eval_2
code: s(:call, nil, :instance_eval, s(:dstr, "interpolated ", s(:evstr, s(:call, nil, :string)), s(:str, " - warning"))),
user_input: nil
end

def test_plain_marshal_load_weak_warning
assert_warning check_name: "Deserialize",
type: :warning,
warning_code: 25,
fingerprint: "458ac9bba693eae0b1d311627d59101dceac803c578bd1da7d808cb333c75068",
warning_type: "Remote Code Execution",
line: 6,
message: /^Use\ of\ `Marshal\.load`\ may\ be\ dangerous/,
confidence: 2,
relative_path: "app/controllers/application_controller.rb",
code: s(:call, s(:const, :Marshal), :load, s(:call, nil, :something)),
user_input: nil
end
end

0 comments on commit 5f37891

Please sign in to comment.