Add new check for weak RSA keys and padding modes

Fixes #1736

lib/brakeman/checks/check_weak_rsa_key.rb

@@ -0,0 +1,77 @@
+require 'brakeman/checks/base_check'
+
+class Brakeman::CheckWeakRSAKey < Brakeman::BaseCheck
+  Brakeman::Checks.add_optional self
+
+  @description = "Checks for weak uses RSA keys"
+
+  def run_check
+    tracker.find_call(targets: [:'OpenSSL::PKey::RSA'], method: [:new, :generate], nested: true).each do |result|
+      check_key_size(result)
+    end
+
+    tracker.find_call(targets: [:'OpenSSL::PKey::RSA.new'], method: [:public_encrypt, :public_decrypt, :private_encrypt, :private_decrypt], nested: true).each do |result|
+      check_padding(result)
+    end
+  end
+
+  def check_key_size result
+    return unless original? result
+
+    first_arg = result[:call].first_arg
+
+    if number? first_arg
+      key_size = first_arg.value
+
+      if key_size < 1024
+        confidence = :high
+        message = msg("RSA key with size ", msg_code(key_size.to_s), " is considered very weak. Use at least 2048 bit key size")
+      elsif key_size < 2048
+        confidence = :medium
+        message = msg("RSA key with size ", msg_code(key_size.to_s), " is considered weak. Use at least 2048 bit key size")
+      else
+        return
+      end
+
+      warn result: result,
+        warning_type: "Weak Cryptography",
+        warning_code: :small_rsa_key_size,
+        message: message,
+        confidence: confidence,
+        user_input: first_arg,
+        cwe_id: [326]
+    end
+  end
+
+  PKCS1_PADDING = s(:const, :PKCS1_PADDING).freeze
+  NO_PADDING = s(:const, :NO_PADDING).freeze
+
+  def check_padding result
+    return unless original? result
+
+    padding_arg = result[:call].second_arg
+
+    case padding_arg 
+    when PKCS1_PADDING, nil
+      message = "Use of insecure padding mode PKCS1 (default if not specified), which is known to be insecure" 
+
+      warn result: result,
+        warning_type: "Weak Cryptography",
+        warning_code: :insecure_rsa_padding_mode,
+        message: message,
+        confidence: :high,
+        user_input: padding_arg,
+        cwe_id: [780]
+    when NO_PADDING
+      message = "No padding mode used for RSA key. A safe padding mode should be specified for RSA keys" 
+
+      warn result: result,
+        warning_type: "Weak Cryptography",
+        warning_code: :missing_rsa_padding_mode,
+        message: message,
+        confidence: :high,
+        user_input: padding_arg,
+        cwe_id: [780]
+    end
+  end
+end

lib/brakeman/warning_codes.rb

@@ -127,6 +127,9 @@ module Brakeman::WarningCodes
     :pending_eol_ruby => 123,
     :CVE_2022_32209 => 124,
     :pathname_traversal => 125,
+    :insecure_rsa_padding_mode => 126,
+    :missing_rsa_padding_mode => 127,
+    :small_rsa_key_size => 128,
 
     :custom_check => 9090,
   }

test/apps/rails7/lib/some_lib.rb

@@ -0,0 +1,20 @@
+class SomeLib
+  def some_rsa_encrypting
+    public_key = OpenSSL::PKey::RSA.new("grab the public 4096 bit key")
+    encrypted = Base64.encode64(public_key.public_encrypt(payload.to_json)) # Weak padding mode default
+    public_key.private_decrypt(Base64.decode64(encrypted)) # Weak padding mode default
+  end
+
+  def some_more_rsa_padding_modes
+    public_key = OpenSSL::PKey::RSA.new("grab the public 4096 bit key")
+    public_key.public_decrypt(data, PKCS1_PADDING)
+    public_key.private_encrypt(data, NO_PADDING)
+    public_key.private_encrypt(data, SSLV23_PADDING)
+  end
+
+  def small_rsa_keys 
+    OpenSSL::PKey::RSA.generate(512) # Very weak
+    OpenSSL::PKey::RSA.new(1024) # Weak
+    OpenSSL::PKey::RSA.new(2048) # Okay
+  end
+end

test/tests/rails7.rb

@@ -13,7 +13,7 @@ def expected
       :controller => 0,
       :model => 0,
       :template => 0,
-      :warning => 4
+      :warning => 10
     }
   end
 
@@ -72,6 +72,90 @@ def test_redirect_to_last
       user_input: s(:call, s(:const, :User), :last!)
   end
 
+  def test_weak_cryptography_1
+    assert_warning check_name: "WeakRSAKey",
+      type: :warning,
+      warning_code: 128,
+      fingerprint: "4c4db18f4142dac0b271136f6bcf8bee08f0585bd9640676a12cdb80b1d7f02d",
+      warning_type: "Weak Cryptography",
+      line: 16,
+      message: /^RSA\ key\ with\ size\ `512`\ is\ considered\ ve/,
+      confidence: 0,
+      relative_path: "lib/some_lib.rb",
+      code: s(:call, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :generate, s(:lit, 512)),
+      user_input: s(:lit, 512)
+  end
+
+  def test_weak_cryptography_2
+    assert_warning check_name: "WeakRSAKey",
+      type: :warning,
+      warning_code: 128,
+      fingerprint: "0f23edef18a0d092581daff053a88b523a56f50c03367907c0167af50d01dec2",
+      warning_type: "Weak Cryptography",
+      line: 17,
+      message: /^RSA\ key\ with\ size\ `1024`\ is\ considered\ w/,
+      confidence: 1,
+      relative_path: "lib/some_lib.rb",
+      code: s(:call, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :new, s(:lit, 1024)),
+      user_input: s(:lit, 1024)
+  end
+
+  def test_weak_cryptography_3
+    assert_warning check_name: "WeakRSAKey",
+      type: :warning,
+      warning_code: 126,
+      fingerprint: "cc38689724cb70423c57d575290423054f0c998a7b897b2985e96da96f51e77e",
+      warning_type: "Weak Cryptography",
+      line: 4,
+      message: /^Use\ of\ insecure\ padding\ mode\ PKCS1\ \(defa/,
+      confidence: 0,
+      relative_path: "lib/some_lib.rb",
+      code: s(:call, s(:call, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :new, s(:str, "grab the public 4096 bit key")), :public_encrypt, s(:call, s(:call, nil, :payload), :to_json)),
+      user_input: nil
+  end
+
+  def test_weak_cryptography_4
+    assert_warning check_name: "WeakRSAKey",
+      type: :warning,
+      warning_code: 126,
+      fingerprint: "53df5254e251a0ab8f6159df3dbdb1a77ff92c96589a213adb9847c2f255a479",
+      warning_type: "Weak Cryptography",
+      line: 5,
+      message: /^Use\ of\ insecure\ padding\ mode\ PKCS1\ \(defa/,
+      confidence: 0,
+      relative_path: "lib/some_lib.rb",
+      code: s(:call, s(:call, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :new, s(:str, "grab the public 4096 bit key")), :private_decrypt, s(:call, s(:const, :Base64), :decode64, s(:call, s(:const, :Base64), :encode64, s(:call, s(:call, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :new, s(:str, "grab the public 4096 bit key")), :public_encrypt, s(:call, s(:call, nil, :payload), :to_json))))),
+      user_input: nil
+  end
+
+  def test_weak_cryptography_5
+    assert_warning check_name: "WeakRSAKey",
+      type: :warning,
+      warning_code: 126,
+      fingerprint: "aa734fa685d04ea9e8519785123aa8a5342342b86aa77a363bcd2754b951433b",
+      warning_type: "Weak Cryptography",
+      line: 10,
+      message: /^Use\ of\ insecure\ padding\ mode\ PKCS1\ \(defa/,
+      confidence: 0,
+      relative_path: "lib/some_lib.rb",
+      code: s(:call, s(:call, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :new, s(:str, "grab the public 4096 bit key")), :public_decrypt, s(:call, nil, :data), s(:const, :PKCS1_PADDING)),
+      user_input: s(:const, :PKCS1_PADDING)
+  end
+
+  def test_weak_cryptography_6
+    assert_warning check_name: "WeakRSAKey",
+      type: :warning,
+      warning_code: 127,
+      fingerprint: "7a65fbcb29780f39bc03dfe6db0ed9959710180e705393e915bbee915c240751",
+      warning_type: "Weak Cryptography",
+      line: 11,
+      message: /^No\ padding\ mode\ used\ for\ RSA\ key\.\ A\ safe/,
+      confidence: 0,
+      relative_path: "lib/some_lib.rb",
+      code: s(:call, s(:call, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :new, s(:str, "grab the public 4096 bit key")), :private_encrypt, s(:call, nil, :data), s(:const, :NO_PADDING)),
+      user_input: s(:const, :NO_PADDING)
+  end
+
   def test_cross_site_scripting_CVE_2022_32209_allowed_tags_initializer
     assert_warning check_name: "SanitizeConfigCve",
       type: :warning,