From f891743504ecacade7bf4cf2b3da667390c33820 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Sun, 29 Dec 2024 16:51:21 -0700 Subject: [PATCH] Always warn about deserializing with Marshal --- lib/brakeman/checks/check_deserialize.rb | 5 ++++- .../app/controllers/application_controller.rb | 4 ++++ test/tests/rails8.rb | 16 +++++++++++++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/brakeman/checks/check_deserialize.rb b/lib/brakeman/checks/check_deserialize.rb index 9051b9dfa1..29692227f4 100644 --- a/lib/brakeman/checks/check_deserialize.rb +++ b/lib/brakeman/checks/check_deserialize.rb @@ -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", diff --git a/test/apps/rails8/app/controllers/application_controller.rb b/test/apps/rails8/app/controllers/application_controller.rb index 0d95db22b4..25682d4d3c 100644 --- a/test/apps/rails8/app/controllers/application_controller.rb +++ b/test/apps/rails8/app/controllers/application_controller.rb @@ -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 diff --git a/test/tests/rails8.rb b/test/tests/rails8.rb index 21f6d0f637..62193eff33 100644 --- a/test/tests/rails8.rb +++ b/test/tests/rails8.rb @@ -16,7 +16,7 @@ def expected controller: 0, model: 0, template: 0, - warning: 2 + warning: 3 } end @@ -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