diff --git a/lib/brakeman/checks/check_weak_rsa_key.rb b/lib/brakeman/checks/check_weak_rsa_key.rb new file mode 100644 index 0000000000..dec91cfc28 --- /dev/null +++ b/lib/brakeman/checks/check_weak_rsa_key.rb @@ -0,0 +1,112 @@ +require 'brakeman/checks/base_check' + +class Brakeman::CheckWeakRSAKey < Brakeman::BaseCheck + Brakeman::Checks.add self + + @description = "Checks for weak uses RSA keys" + + def run_check + check_rsa_key_creation + check_rsa_operations + end + + def check_rsa_key_creation + tracker.find_call(targets: [:'OpenSSL::PKey::RSA'], method: [:new, :generate], nested: true).each do |result| + key_size_arg = result[:call].first_arg + check_key_size(result, key_size_arg) + end + + tracker.find_call(targets: [:'OpenSSL::PKey'], method: [:generate_key], nested: true).each do |result| + call = result[:call] + key_type = call.first_arg + options_arg = call.second_arg + + next unless options_arg and hash? options_arg + + if string? key_type and key_type.value.upcase == 'RSA' + key_size_arg = (hash_access(options_arg, :rsa_keygen_bits) || hash_access(options_arg, s(:str, 'rsa_key_gen_bits'))) + check_key_size(result, key_size_arg) + end + end + end + + def check_rsa_operations + tracker.find_call(targets: [:'OpenSSL::PKey::RSA.new'], methods: [:public_encrypt, :public_decrypt, :private_encrypt, :private_decrypt], nested: true).each do |result| + padding_arg = result[:call].second_arg + check_padding(result, padding_arg) + end + + tracker.find_call(targets: [:'OpenSSL::PKey.generate_key'], methods: [:encrypt, :decrypt, :sign, :verify, :sign_raw, :verify_raw], nested: true).each do |result| + call = result[:call] + options_arg = call.last_arg + + if options_arg and hash? options_arg + padding_arg = (hash_access(options_arg, :rsa_padding_mode) || hash_access(options_arg, s(:str, 'rsa_padding_mode'))) + else + padding_arg = nil + end + + check_padding(result, padding_arg) + end + end + + def check_key_size result, key_size_arg + return unless number? key_size_arg + return unless original? result + + key_size = key_size_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: key_size_arg, + cwe_id: [326] + end + + PKCS1_PADDING = s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :PKCS1_PADDING).freeze + PKCS1_PADDING_STR = s(:str, 'pkcs1').freeze + SSLV23_PADDING = s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :SSLV23_PADDING).freeze + SSLV23_PADDING_STR = s(:str, 'sslv23').freeze + NO_PADDING = s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :NO_PADDING).freeze + NO_PADDING_STR = s(:str, 'none').freeze + + def check_padding result, padding_arg + return unless original? result + + if string? padding_arg + padding_arg = padding_arg.deep_clone(padding_arg.line) + padding_arg.value.downcase! + end + + case padding_arg + when PKCS1_PADDING, PKCS1_PADDING_STR, nil + message = "Use of padding mode PKCS1 (default if not specified), which is known to be insecure. Use OAEP instead" + when SSLV23_PADDING, SSLV23_PADDING_STR + message = "Use of padding mode SSLV23 for RSA key, which is only useful for outdated versions of SSL. Use OAEP instead" + when NO_PADDING, NO_PADDING_STR + message = "No padding mode used for RSA key. A safe padding mode (OAEP) should be specified for RSA keys" + else + return + end + + warn result: result, + warning_type: "Weak Cryptography", + warning_code: :insecure_rsa_padding_mode, + message: message, + confidence: :high, + user_input: padding_arg, + cwe_id: [780] + end +end diff --git a/lib/brakeman/warning_codes.rb b/lib/brakeman/warning_codes.rb index 29938f9cd3..dacafb8c8f 100644 --- a/lib/brakeman/warning_codes.rb +++ b/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, } diff --git a/test/apps/rails7/lib/some_lib.rb b/test/apps/rails7/lib/some_lib.rb new file mode 100644 index 0000000000..08733ed866 --- /dev/null +++ b/test/apps/rails7/lib/some_lib.rb @@ -0,0 +1,31 @@ +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, OpenSSL::PKey::RSA::PKCS1_PADDING) + public_key.private_encrypt(data, OpenSSL::PKey::RSA::NO_PADDING) + public_key.private_encrypt(data, OpenSSL::PKey::RSA::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 + + def pky_api + weak_rsa = OpenSSL::PKey.generate_key("rsa", rsa_keygen_bits: 1024) # Medium warning about key size + weak_encrypted = weak_rsa.encrypt("data", "rsa_padding_mode" => "pkcs1") + weak_encrypted = weak_rsa.decrypt("data", "rsa_padding_mode" => "oaep") + weak_signature_digest = weak_rsa.sign("SHA256", "data", rsa_padding_mode: "PKCS1") + weak_rsa.verify("SHA256", "data", rsa_padding_mode: "none") + weak_rsa.sign_raw(nil, "data", rsa_padding_mode: "none") + weak_rsa.verify_raw(nil, "data", rsa_padding_mode: "none") + weak_rsa.encrypt("data") # default is also pkcs1 + end +end diff --git a/test/tests/rails7.rb b/test/tests/rails7.rb index 2ba0a98862..adb663b6c8 100644 --- a/test/tests/rails7.rb +++ b/test/tests/rails7.rb @@ -13,7 +13,7 @@ def expected :controller => 0, :model => 0, :template => 0, - :warning => 4 + :warning => 18 } end @@ -72,6 +72,202 @@ 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: 128, + fingerprint: "74dd38e229f0343ce80891b7530c4ecf3446c2f214917f70a1044006c885a6b0", + warning_type: "Weak Cryptography", + line: 22, + message: /^RSA\ key\ with\ size\ `1024`\ is\ considered\ w/, + confidence: 1, + relative_path: "lib/some_lib.rb", + code: s(:call, s(:colon2, s(:const, :OpenSSL), :PKey), :generate_key, s(:str, "rsa"), s(:hash, s(:lit, :rsa_keygen_bits), s(:lit, 1024))), + user_input: s(:lit, 1024) + end + + def test_weak_cryptography_4 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "cc38689724cb70423c57d575290423054f0c998a7b897b2985e96da96f51e77e", + warning_type: "Weak Cryptography", + line: 4, + message: /^Use\ of\ padding\ mode\ PKCS1\ \(default\ if\ no/, + 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_5 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "53df5254e251a0ab8f6159df3dbdb1a77ff92c96589a213adb9847c2f255a479", + warning_type: "Weak Cryptography", + line: 5, + message: /^Use\ of\ padding\ mode\ PKCS1\ \(default\ if\ no/, + 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_6 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "c8a3c3c409f64eae926ce9b60d85d243f86bc8448d1ba7b5880f192eb54089d7", + warning_type: "Weak Cryptography", + line: 10, + message: /^Use\ of\ padding\ mode\ PKCS1\ \(default\ if\ no/, + 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(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :PKCS1_PADDING)), + user_input: s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :PKCS1_PADDING) + end + + def test_weak_cryptography_7 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "bf3a313e24667f5839385b4ad0e90bc51a4f6bf8b489dab152c03242641ebad9", + 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(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :NO_PADDING)), + user_input: s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :NO_PADDING) + end + + def test_weak_cryptography_8 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "7692aefd6fc53891734025f079ac062bf5b4ca69d1447f53de8f7e0cd389ae19", + warning_type: "Weak Cryptography", + line: 12, + message: /^Use\ of\ padding\ mode\ SSLV23\ for\ RSA\ key,\ /, + 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(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :SSLV23_PADDING)), + user_input: s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :SSLV23_PADDING) + end + + def test_weak_cryptography_9 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "386909718cfc8427e4509912c7c22b0f99ce2e052bb505ccfe6b400e3fd21632", + warning_type: "Weak Cryptography", + line: 23, + message: /^Use\ of\ padding\ mode\ PKCS1\ \(default\ if\ no/, + confidence: 0, + relative_path: "lib/some_lib.rb", + code: s(:call, s(:call, s(:colon2, s(:const, :OpenSSL), :PKey), :generate_key, s(:str, "rsa"), s(:hash, s(:lit, :rsa_keygen_bits), s(:lit, 1024))), :encrypt, s(:str, "data"), s(:hash, s(:str, "rsa_padding_mode"), s(:str, "pkcs1"))), + user_input: s(:str, "pkcs1") + end + + def test_weak_cryptography_10 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "3454ec09e3264042301160253d0846296f1334fcb33252edd5d5c41cc3712ab3", + warning_type: "Weak Cryptography", + line: 25, + message: /^Use\ of\ padding\ mode\ PKCS1\ \(default\ if\ no/, + confidence: 0, + relative_path: "lib/some_lib.rb", + code: s(:call, s(:call, s(:colon2, s(:const, :OpenSSL), :PKey), :generate_key, s(:str, "rsa"), s(:hash, s(:lit, :rsa_keygen_bits), s(:lit, 1024))), :sign, s(:str, "SHA256"), s(:str, "data"), s(:hash, s(:lit, :rsa_padding_mode), s(:str, "pkcs1"))), + user_input: s(:str, "pkcs1") + end + + def test_weak_cryptography_11 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "0b6b1f354c2380be841134447c315a24c2919d61fbb4de51af3dafc66e2144c3", + warning_type: "Weak Cryptography", + line: 26, + 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(:const, :OpenSSL), :PKey), :generate_key, s(:str, "rsa"), s(:hash, s(:lit, :rsa_keygen_bits), s(:lit, 1024))), :verify, s(:str, "SHA256"), s(:str, "data"), s(:hash, s(:lit, :rsa_padding_mode), s(:str, "none"))), + user_input: s(:str, "none") + end + + def test_weak_cryptography_12 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "cf7d2b90d591ca7a442992caf39b858c4e599c9f2f4d82fa09e40b250f9c8e78", + warning_type: "Weak Cryptography", + line: 27, + 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(:const, :OpenSSL), :PKey), :generate_key, s(:str, "rsa"), s(:hash, s(:lit, :rsa_keygen_bits), s(:lit, 1024))), :sign_raw, s(:nil), s(:str, "data"), s(:hash, s(:lit, :rsa_padding_mode), s(:str, "none"))), + user_input: s(:str, "none") + end + + def test_weak_cryptography_13 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "6a9835fa708e6f92797c4c1164b32446fe028672ba7ad652d3a474072658e271", + warning_type: "Weak Cryptography", + line: 28, + 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(:const, :OpenSSL), :PKey), :generate_key, s(:str, "rsa"), s(:hash, s(:lit, :rsa_keygen_bits), s(:lit, 1024))), :verify_raw, s(:nil), s(:str, "data"), s(:hash, s(:lit, :rsa_padding_mode), s(:str, "none"))), + user_input: s(:str, "none") + end + + def test_weak_cryptography_14 + assert_warning check_name: "WeakRSAKey", + type: :warning, + warning_code: 126, + fingerprint: "a7c85f295d9ea5356afbdf9165eb5bcfb892646f5f9a5a73b514a835456b419b", + warning_type: "Weak Cryptography", + line: 29, + message: /^Use\ of\ padding\ mode\ PKCS1\ \(default\ if\ no/, + confidence: 0, + relative_path: "lib/some_lib.rb", + code: s(:call, s(:call, s(:colon2, s(:const, :OpenSSL), :PKey), :generate_key, s(:str, "rsa"), s(:hash, s(:lit, :rsa_keygen_bits), s(:lit, 1024))), :encrypt, s(:str, "data")), + user_input: nil + end + def test_cross_site_scripting_CVE_2022_32209_allowed_tags_initializer assert_warning check_name: "SanitizeConfigCve", type: :warning,