From 29959595204a739631731528cdc2a82dd83e636e Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Sat, 22 Oct 2022 14:05:12 -0700 Subject: [PATCH 1/6] Add new check for weak RSA keys and padding modes Fixes #1736 --- lib/brakeman/checks/check_weak_rsa_key.rb | 77 ++++++++++++++++++++ lib/brakeman/warning_codes.rb | 3 + test/apps/rails7/lib/some_lib.rb | 20 ++++++ test/tests/rails7.rb | 86 ++++++++++++++++++++++- 4 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 lib/brakeman/checks/check_weak_rsa_key.rb create mode 100644 test/apps/rails7/lib/some_lib.rb 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..134d516959 --- /dev/null +++ b/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 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 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 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..5b4846a58b --- /dev/null +++ b/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 diff --git a/test/tests/rails7.rb b/test/tests/rails7.rb index 2ba0a98862..6c5be02b63 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 => 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, From 78c1a9e2f577912bcb874154bf5dc9f83b1901ae Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Mon, 24 Oct 2022 15:50:46 -0700 Subject: [PATCH 2/6] Fix message in tests --- test/tests/rails7.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/tests/rails7.rb b/test/tests/rails7.rb index 6c5be02b63..b9af72f255 100644 --- a/test/tests/rails7.rb +++ b/test/tests/rails7.rb @@ -107,7 +107,7 @@ def test_weak_cryptography_3 fingerprint: "cc38689724cb70423c57d575290423054f0c998a7b897b2985e96da96f51e77e", warning_type: "Weak Cryptography", line: 4, - message: /^Use\ of\ insecure\ padding\ mode\ PKCS1\ \(defa/, + message: /^Use\ of\ 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)), @@ -121,7 +121,7 @@ def test_weak_cryptography_4 fingerprint: "53df5254e251a0ab8f6159df3dbdb1a77ff92c96589a213adb9847c2f255a479", warning_type: "Weak Cryptography", line: 5, - message: /^Use\ of\ insecure\ padding\ mode\ PKCS1\ \(defa/, + message: /^Use\ of\ 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))))), @@ -135,7 +135,7 @@ def test_weak_cryptography_5 fingerprint: "aa734fa685d04ea9e8519785123aa8a5342342b86aa77a363bcd2754b951433b", warning_type: "Weak Cryptography", line: 10, - message: /^Use\ of\ insecure\ padding\ mode\ PKCS1\ \(defa/, + message: /^Use\ of\ 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)), From 394f14b3ac37ebf3b55f8850a47bb85cc64070f7 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Mon, 24 Oct 2022 23:36:59 -0700 Subject: [PATCH 3/6] Fix padding constants --- lib/brakeman/checks/check_weak_rsa_key.rb | 4 ++-- test/apps/rails7/lib/some_lib.rb | 6 +++--- test/tests/rails7.rb | 16 ++++++++-------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/brakeman/checks/check_weak_rsa_key.rb b/lib/brakeman/checks/check_weak_rsa_key.rb index 134d516959..ef54e562df 100644 --- a/lib/brakeman/checks/check_weak_rsa_key.rb +++ b/lib/brakeman/checks/check_weak_rsa_key.rb @@ -43,8 +43,8 @@ def check_key_size result end end - PKCS1_PADDING = s(:const, :PKCS1_PADDING).freeze - NO_PADDING = s(:const, :NO_PADDING).freeze + PKCS1_PADDING = s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :PKCS1_PADDING).freeze + NO_PADDING = s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :NO_PADDING).freeze def check_padding result return unless original? result diff --git a/test/apps/rails7/lib/some_lib.rb b/test/apps/rails7/lib/some_lib.rb index 5b4846a58b..b9c98d7e75 100644 --- a/test/apps/rails7/lib/some_lib.rb +++ b/test/apps/rails7/lib/some_lib.rb @@ -7,9 +7,9 @@ def some_rsa_encrypting 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) + 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 diff --git a/test/tests/rails7.rb b/test/tests/rails7.rb index b9af72f255..9d6dc7b4a0 100644 --- a/test/tests/rails7.rb +++ b/test/tests/rails7.rb @@ -132,28 +132,28 @@ def test_weak_cryptography_5 assert_warning check_name: "WeakRSAKey", type: :warning, warning_code: 126, - fingerprint: "aa734fa685d04ea9e8519785123aa8a5342342b86aa77a363bcd2754b951433b", + fingerprint: "c8a3c3c409f64eae926ce9b60d85d243f86bc8448d1ba7b5880f192eb54089d7", warning_type: "Weak Cryptography", line: 10, - message: /^Use\ of\ padding\ mode\ PKCS1\ \(defa/, + 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(:const, :PKCS1_PADDING)), - user_input: s(:const, :PKCS1_PADDING) + 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_6 + def test_weak_cryptography_6 assert_warning check_name: "WeakRSAKey", type: :warning, warning_code: 127, - fingerprint: "7a65fbcb29780f39bc03dfe6db0ed9959710180e705393e915bbee915c240751", + fingerprint: "47462db72333e2287d0b3670295f875700e85f516b4276ec5acf2f99f3809b04", 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) + 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_cross_site_scripting_CVE_2022_32209_allowed_tags_initializer From f2ccd7d3942a3369a8f77e00d6c783b6d026ccd7 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Mon, 7 Nov 2022 23:14:21 -0800 Subject: [PATCH 4/6] Several more RSA scenarios --- lib/brakeman/checks/check_weak_rsa_key.rb | 120 +++++++++++++--------- test/apps/rails7/lib/some_lib.rb | 11 ++ 2 files changed, 85 insertions(+), 46 deletions(-) diff --git a/lib/brakeman/checks/check_weak_rsa_key.rb b/lib/brakeman/checks/check_weak_rsa_key.rb index ef54e562df..bbeb38a564 100644 --- a/lib/brakeman/checks/check_weak_rsa_key.rb +++ b/lib/brakeman/checks/check_weak_rsa_key.rb @@ -7,71 +7,99 @@ class Brakeman::CheckWeakRSAKey < Brakeman::BaseCheck def run_check tracker.find_call(targets: [:'OpenSSL::PKey::RSA'], method: [:new, :generate], nested: true).each do |result| - check_key_size(result) + key_size_arg = result[:call].first_arg + check_key_size(result, key_size_arg) 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 + 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 - def check_key_size result - return unless original? result + 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 - first_arg = result[:call].first_arg + 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 - if number? first_arg - key_size = first_arg.value + 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 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") + 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 - return + padding_arg = nil 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] + 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 + def check_padding result, padding_arg return unless original? result - padding_arg = result[:call].second_arg + if string? padding_arg + padding_arg = padding_arg.deep_clone(padding_arg.line) + padding_arg.value.downcase! + end case padding_arg - when PKCS1_PADDING, nil - message = "Use of 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] + 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/test/apps/rails7/lib/some_lib.rb b/test/apps/rails7/lib/some_lib.rb index b9c98d7e75..08733ed866 100644 --- a/test/apps/rails7/lib/some_lib.rb +++ b/test/apps/rails7/lib/some_lib.rb @@ -17,4 +17,15 @@ def small_rsa_keys 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 From 69aec858217b6f4dc135e9e829d2672ae28e13cf Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Thu, 10 Nov 2022 08:45:23 -0800 Subject: [PATCH 5/6] Update RSA tests --- test/tests/rails7.rb | 128 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 120 insertions(+), 8 deletions(-) diff --git a/test/tests/rails7.rb b/test/tests/rails7.rb index 9d6dc7b4a0..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 => 10 + :warning => 18 } end @@ -101,34 +101,48 @@ def test_weak_cryptography_2 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\ \(defa/, + 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_4 + 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\ \(defa/, + 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_5 + def test_weak_cryptography_6 assert_warning check_name: "WeakRSAKey", type: :warning, warning_code: 126, @@ -142,11 +156,11 @@ def test_weak_cryptography_5 user_input: s(:colon2, s(:colon2, s(:colon2, s(:const, :OpenSSL), :PKey), :RSA), :PKCS1_PADDING) end - def test_weak_cryptography_6 + def test_weak_cryptography_7 assert_warning check_name: "WeakRSAKey", type: :warning, - warning_code: 127, - fingerprint: "47462db72333e2287d0b3670295f875700e85f516b4276ec5acf2f99f3809b04", + warning_code: 126, + fingerprint: "bf3a313e24667f5839385b4ad0e90bc51a4f6bf8b489dab152c03242641ebad9", warning_type: "Weak Cryptography", line: 11, message: /^No\ padding\ mode\ used\ for\ RSA\ key\.\ A\ safe/, @@ -156,6 +170,104 @@ def test_weak_cryptography_6 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, From 4f4c9c07a449a4857e0093d86a630be9c06aed74 Mon Sep 17 00:00:00 2001 From: Justin Collins Date: Thu, 10 Nov 2022 09:22:09 -0800 Subject: [PATCH 6/6] Move code around --- lib/brakeman/checks/check_weak_rsa_key.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/brakeman/checks/check_weak_rsa_key.rb b/lib/brakeman/checks/check_weak_rsa_key.rb index bbeb38a564..dec91cfc28 100644 --- a/lib/brakeman/checks/check_weak_rsa_key.rb +++ b/lib/brakeman/checks/check_weak_rsa_key.rb @@ -6,6 +6,11 @@ class Brakeman::CheckWeakRSAKey < Brakeman::BaseCheck @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) @@ -23,7 +28,9 @@ def run_check 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)