diff --git a/config/_default_shared.yml b/config/_default_shared.yml index bca634b..6920374 100644 --- a/config/_default_shared.yml +++ b/config/_default_shared.yml @@ -10,6 +10,9 @@ Bundler/DuplicatedGem: Bundler/OrderedGems: Enabled: true +GitHub/InsecureHashAlgorithm: + Enabled: true + Layout/BlockAlignment: Enabled: true diff --git a/lib/rubocop/cop/github.rb b/lib/rubocop/cop/github.rb index 942fd34..e978c2b 100644 --- a/lib/rubocop/cop/github.rb +++ b/lib/rubocop/cop/github.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "rubocop/cop/github/insecure_hash_algorithm" require "rubocop/cop/github/rails_application_record" require "rubocop/cop/github/rails_controller_render_action_symbol" require "rubocop/cop/github/rails_controller_render_literal" diff --git a/lib/rubocop/cop/github/insecure_hash_algorithm.rb b/lib/rubocop/cop/github/insecure_hash_algorithm.rb new file mode 100644 index 0000000..d452730 --- /dev/null +++ b/lib/rubocop/cop/github/insecure_hash_algorithm.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require "rubocop" + +module RuboCop + module Cop + module GitHub + class InsecureHashAlgorithm < Cop + MSG = "This hash function is not allowed" + UUID_V3_MSG = "uuid_v3 uses MD5, which is not allowed" + UUID_V5_MSG = "uuid_v5 uses SHA1, which is not allowed" + + # Matches constants like these: + # Digest::MD5 + # OpenSSL::Digest::MD5 + def_node_matcher :insecure_const?, <<-PATTERN + (const (const _ :Digest) #insecure_algorithm?) + PATTERN + + # Matches calls like these: + # Digest.new('md5') + # Digest.hexdigest('md5', 'str') + # OpenSSL::Digest.new('md5') + # OpenSSL::Digest.hexdigest('md5', 'str') + # OpenSSL::Digest::Digest.new('md5') + # OpenSSL::Digest::Digest.hexdigest('md5', 'str') + # OpenSSL::Digest::Digest.new(:MD5) + # OpenSSL::Digest::Digest.hexdigest(:MD5, 'str') + def_node_matcher :insecure_digest?, <<-PATTERN + (send + (const _ {:Digest :HMAC}) + #not_just_encoding? + #insecure_algorithm? + ...) + PATTERN + + # Matches calls like "Digest(:MD5)". + def_node_matcher :insecure_hash_lookup?, <<-PATTERN + (send _ :Digest #insecure_algorithm?) + PATTERN + + # Matches calls like "OpenSSL::HMAC.new(secret, hash)" + def_node_matcher :openssl_hmac_new?, <<-PATTERN + (send (const (const _ :OpenSSL) :HMAC) :new ...) + PATTERN + + # Matches calls like "OpenSSL::HMAC.new(secret, 'sha1')" + def_node_matcher :openssl_hmac_new_insecure?, <<-PATTERN + (send (const (const _ :OpenSSL) :HMAC) :new _ #insecure_algorithm?) + PATTERN + + # Matches Rails's Digest::UUID. + def_node_matcher :digest_uuid?, <<-PATTERN + (const (const _ :Digest) :UUID) + PATTERN + + def_node_matcher :uuid_v3?, <<-PATTERN + (send (const _ :UUID) :uuid_v3 ...) + PATTERN + + def_node_matcher :uuid_v5?, <<-PATTERN + (send (const _ :UUID) :uuid_v5 ...) + PATTERN + + def insecure_algorithm?(val) + return false if val == :Digest # Don't match "Digest::Digest". + case alg_name(val) + when *allowed_hash_functions + false + when Symbol + # can't figure this one out, it's nil or a var or const. + false + else + true + end + end + + def not_just_encoding?(val) + !just_encoding?(val) + end + + def just_encoding?(val) + val == :hexencode || val == :bubblebabble + end + + # Built-in hash functions are listed in these docs: + # https://ruby-doc.org/stdlib-2.7.0/libdoc/digest/rdoc/Digest.html + # https://ruby-doc.org/stdlib-2.7.0/libdoc/openssl/rdoc/OpenSSL/Digest.html + DEFAULT_ALLOWED = %w[ + SHA256 + SHA384 + SHA512 + ].freeze + + def allowed_hash_functions + @allowed_algorithms ||= cop_config.fetch("Allowed", DEFAULT_ALLOWED).map(&:downcase) + end + + def alg_name(val) + return :nil if val.nil? + return val.to_s.downcase unless val.is_a?(RuboCop::AST::Node) + case val.type + when :sym, :str + val.children.first.to_s.downcase + else + val.type + end + end + + def on_const(const_node) + if insecure_const?(const_node) && !digest_uuid?(const_node) + add_offense(const_node, message: MSG) + end + end + + def on_send(send_node) + case + when uuid_v3?(send_node) + unless allowed_hash_functions.include?("md5") + add_offense(send_node, message: UUID_V3_MSG) + end + when uuid_v5?(send_node) + unless allowed_hash_functions.include?("sha1") + add_offense(send_node, message: UUID_V5_MSG) + end + when openssl_hmac_new?(send_node) + if openssl_hmac_new_insecure?(send_node) + add_offense(send_node, message: MSG) + end + when insecure_digest?(send_node) + add_offense(send_node, message: MSG) + when insecure_hash_lookup?(send_node) + add_offense(send_node, message: MSG) + end + end + end + end + end +end diff --git a/test/test_insecure_hash_algorithm.rb b/test/test_insecure_hash_algorithm.rb new file mode 100644 index 0000000..ee18db7 --- /dev/null +++ b/test/test_insecure_hash_algorithm.rb @@ -0,0 +1,443 @@ +# frozen_string_literal: true + +require_relative "./cop_test" +require "minitest/autorun" +require "rubocop/cop/github/insecure_hash_algorithm" + +class TestInsecureHashAlgorithm < CopTest + def cop_class + RuboCop::Cop::GitHub::InsecureHashAlgorithm + end + + def make_cop(allowed:) + config = RuboCop::Config.new({"GitHub/InsecureHashAlgorithm" => {"Allowed" => allowed}}) + cop_class.new(config) + end + + def test_benign_apis + investigate(cop, <<-RUBY) + class Something + def hexencode_the_string_md5 + Digest.hexencode('anything') + end + def bubblebabble_the_string_md5 + Digest.bubblebabble('anything') + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_openssl_hmac_new_sha256_indirect + investigate(cop, <<-RUBY) + class Something + HASH = OpenSSL::Digest::SHA256 + + attr :secret + + def secret_hmac + OpenSSL::HMAC.new(self.secret, HASH) + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_openssl_hmac_new_sha1 + investigate(cop, <<-RUBY) + class Something + attr :secret + + def secret_hmac + OpenSSL::HMAC.new(self.secret, 'sha1') + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_digest_method_md5_str + investigate(cop, <<-RUBY) + class Something + def h + Digest('md5') + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_digest_method_md5_symbol + investigate(cop, <<-RUBY) + class Something + def h + Digest(:MD5) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_digest_method_sha256_str + investigate(cop, <<-RUBY) + class Something + def h + Digest('sha256') + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_digest_method_sha256_symbol + investigate(cop, <<-RUBY) + class Something + def h + Digest('sha256') + end + + def h + Digest(:SHA256) + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_alias_for_digest_md5 + investigate(cop, <<-RUBY) + class Something + HASH = Digest::MD5 + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_alias_for_openssl_digest_md5 + investigate(cop, <<-RUBY) + class Something + HASH = OpenSSL::Digest::MD5 + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_alias_for_digest_sha1 + investigate(cop, <<-RUBY) + class Something + HASH = Digest::SHA1 + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_alias_for_openssl_digest_sha1 + investigate(cop, <<-RUBY) + class Something + HASH = OpenSSL::Digest::SHA1 + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_alias_for_digest_sha256 + investigate(cop, <<-RUBY) + class Something + HASH = Digest::SHA256 + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_alias_for_openssl_digest_sha256 + investigate(cop, <<-RUBY) + class Something + HASH = OpenSSL::Digest::SHA256 + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_hexdigest_on_random_class + investigate(cop, <<-RUBY) + class Something + def something(str) + HASH.hexdigest(str) + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_md5_hexdigest + investigate(cop, <<-RUBY) + class Something + def something(str) + Digest::MD5.hexdigest(str) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_md5_hexdigest + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest::MD5.hexdigest(str) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_md5_digest_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest.digest("MD5", str) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_sha1_digest_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest.digest("SHA1", str) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_sha1_hexdigest_by_name_mixed_case + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest.hexdigest("Sha1", str) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_sha256_digest_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest.digest("SHA256", str) + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_openssl_md5_hmac_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::HMAC.hexdigest('md5', str) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_sha1_hmac_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::HMAC.hexdigest('sha1', str) + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_sha256_hmac_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::HMAC.hexdigest('sha256', str) + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_openssl_md5_digest_instance_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest::Digest.new('md5') + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_sha1_digest_instance_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest::Digest.new('sha1') + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::MSG, cop.offenses.first.message + end + + def test_openssl_sha256_digest_instance_by_name + investigate(cop, <<-RUBY) + class Something + def something(str) + OpenSSL::Digest::Digest.new('sha256') + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_uuid_from_hash + investigate(cop, <<-RUBY) + class Something + def uuid + # I want to demonstrate that uuid_from_hash isn't a trigger, + # even though I don't know if it's even valid to use SHA256. + Digest::UUID.uuid_from_hash(Digest::SHA256, 'abc', 'def') + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_uuid_v3 + investigate(cop, <<-RUBY) + class Something + def uuid + Digest::UUID.uuid_v3('anything', 'anything') + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::UUID_V3_MSG, cop.offenses.first.message + end + + def test_uuid_v3_with_md5_allowed + cop = make_cop(allowed: %w[MD5]) + investigate(cop, <<-RUBY) + class Something + def uuid + Digest::UUID.uuid_v3('anything', 'anything') + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_uuid_v4 + investigate(cop, <<-RUBY) + class Something + def uuid + Digest::UUID.uuid_v4 + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_uuid_v5 + investigate(cop, <<-RUBY) + class Something + def uuid + Digest::UUID.uuid_v5('anything', 'anything') + end + end + RUBY + + assert_equal 1, cop.offenses.count + assert_equal cop_class::UUID_V5_MSG, cop.offenses.first.message + end + + def test_uuid_v5_with_sha1_allowed + cop = make_cop(allowed: %w[SHA1]) + investigate(cop, <<-RUBY) + class Something + def uuid + Digest::UUID.uuid_v5('anything', 'anything') + end + end + RUBY + + assert_equal 0, cop.offenses.count + end + + def test_allow_sha512_only + cop = make_cop(allowed: %w[SHA512]) + investigate(cop, <<-RUBY) + class Something + HASH = Digest::SHA256 + end + RUBY + assert_equal 1, cop.offenses.count + end + + def test_allow_lots_of_hashes + cop = make_cop(allowed: %w[SHA1 SHA256 SHA384 SHA512]) + investigate(cop, <<-RUBY) + class Something + HASH = Digest::SHA1 + end + RUBY + assert_equal 0, cop.offenses.count + end +end