Skip to content

Commit e51e5c2

Browse files
committed
Add special checks for rails's Digest::UUID
https://api.rubyonrails.org/classes/Digest/UUID.html uuid_v3 uses MD5, so it's only allowed when MD5 is in the "Allowed" list. uuid_v5 uses SHA1, so it's only allowed when SHA1 is in the "Allowed" list.
1 parent f4abd51 commit e51e5c2

File tree

2 files changed

+101
-12
lines changed

2 files changed

+101
-12
lines changed

lib/rubocop/cop/github/insecure_hash_algorithm.rb

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ module Cop
77
module GitHub
88
class InsecureHashAlgorithm < Cop
99
MSG = "This hash function is not allowed"
10+
UUID_V3_MSG = "uuid_v3 uses MD5, which is not allowed"
11+
UUID_V5_MSG = "uuid_v5 uses SHA1, which is not allowed"
1012

1113
# Matches constants like these:
1214
# Digest::MD5
@@ -47,6 +49,19 @@ class InsecureHashAlgorithm < Cop
4749
(send (const (const _ :OpenSSL) :HMAC) :new _ #insecure_algorithm?)
4850
PATTERN
4951

52+
# Matches Rails's Digest::UUID.
53+
def_node_matcher :digest_uuid?, <<-PATTERN
54+
(const (const _ :Digest) :UUID)
55+
PATTERN
56+
57+
def_node_matcher :uuid_v3?, <<-PATTERN
58+
(send (const _ :UUID) :uuid_v3 ...)
59+
PATTERN
60+
61+
def_node_matcher :uuid_v5?, <<-PATTERN
62+
(send (const _ :UUID) :uuid_v5 ...)
63+
PATTERN
64+
5065
def insecure_algorithm?(val)
5166
return false if val == :Digest # Don't match "Digest::Digest".
5267
case alg_name(val)
@@ -93,13 +108,21 @@ def alg_name(val)
93108
end
94109

95110
def on_const(const_node)
96-
if insecure_const?(const_node)
111+
if insecure_const?(const_node) && !digest_uuid?(const_node)
97112
add_offense(const_node, message: MSG)
98113
end
99114
end
100115

101116
def on_send(send_node)
102117
case
118+
when uuid_v3?(send_node)
119+
unless allowed_hash_functions.include?("md5")
120+
add_offense(send_node, message: UUID_V3_MSG)
121+
end
122+
when uuid_v5?(send_node)
123+
unless allowed_hash_functions.include?("sha1")
124+
add_offense(send_node, message: UUID_V5_MSG)
125+
end
103126
when openssl_hmac_new?(send_node)
104127
if openssl_hmac_new_insecure?(send_node)
105128
add_offense(send_node, message: MSG)

test/test_insecure_hash_algorithm.rb

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ def cop_class
99
RuboCop::Cop::GitHub::InsecureHashAlgorithm
1010
end
1111

12-
def make_cop(config_hash)
13-
config = RuboCop::Config.new({"GitHub/InsecureHashAlgorithm" => config_hash})
12+
def make_cop(allowed:)
13+
config = RuboCop::Config.new({"GitHub/InsecureHashAlgorithm" => {"Allowed" => allowed}})
1414
cop_class.new(config)
1515
end
1616

@@ -343,35 +343,101 @@ def something(str)
343343
assert_equal 0, cop.offenses.count
344344
end
345345

346-
def test_allow_sha512_only
347-
cop = make_cop "Allowed" => %w[SHA512]
346+
def test_uuid_from_hash
348347
investigate(cop, <<-RUBY)
349348
class Something
350-
HASH = Digest::SHA256
349+
def uuid
350+
# I want to demonstrate that uuid_from_hash isn't a trigger,
351+
# even though I don't know if it's even valid to use SHA256.
352+
Digest::UUID.uuid_from_hash(Digest::SHA256, 'abc', 'def')
353+
end
351354
end
352355
RUBY
356+
357+
assert_equal 0, cop.offenses.count
358+
end
359+
360+
def test_uuid_v3
361+
investigate(cop, <<-RUBY)
362+
class Something
363+
def uuid
364+
Digest::UUID.uuid_v3('anything', 'anything')
365+
end
366+
end
367+
RUBY
368+
353369
assert_equal 1, cop.offenses.count
370+
assert_equal cop_class::UUID_V3_MSG, cop.offenses.first.message
354371
end
355372

356-
def test_allow_lots_of_hashes
357-
cop = make_cop "Allowed" => %w[SHA1 SHA256 SHA384 SHA512]
373+
def test_uuid_v3_with_md5_allowed
374+
cop = make_cop(allowed: %w[MD5])
358375
investigate(cop, <<-RUBY)
359376
class Something
360-
HASH = Digest::SHA1
377+
def uuid
378+
Digest::UUID.uuid_v3('anything', 'anything')
379+
end
361380
end
362381
RUBY
382+
383+
assert_equal 0, cop.offenses.count
384+
end
385+
386+
def test_uuid_v4
387+
investigate(cop, <<-RUBY)
388+
class Something
389+
def uuid
390+
Digest::UUID.uuid_v4
391+
end
392+
end
393+
RUBY
394+
363395
assert_equal 0, cop.offenses.count
364396
end
365397

366-
def test_allow_other_digest_types
367-
cop = make_cop "Allowed" => %w[UUID]
398+
def test_uuid_v5
368399
investigate(cop, <<-RUBY)
369400
class Something
370401
def uuid
371-
Digest::UUID.create
402+
Digest::UUID.uuid_v5('anything', 'anything')
372403
end
373404
end
374405
RUBY
406+
407+
assert_equal 1, cop.offenses.count
408+
assert_equal cop_class::UUID_V5_MSG, cop.offenses.first.message
409+
end
410+
411+
def test_uuid_v5_with_sha1_allowed
412+
cop = make_cop(allowed: %w[SHA1])
413+
investigate(cop, <<-RUBY)
414+
class Something
415+
def uuid
416+
Digest::UUID.uuid_v5('anything', 'anything')
417+
end
418+
end
419+
RUBY
420+
421+
assert_equal 0, cop.offenses.count
422+
end
423+
424+
def test_allow_sha512_only
425+
cop = make_cop(allowed: %w[SHA512])
426+
investigate(cop, <<-RUBY)
427+
class Something
428+
HASH = Digest::SHA256
429+
end
430+
RUBY
431+
assert_equal 1, cop.offenses.count
432+
end
433+
434+
def test_allow_lots_of_hashes
435+
cop = make_cop(allowed: %w[SHA1 SHA256 SHA384 SHA512])
436+
investigate(cop, <<-RUBY)
437+
class Something
438+
HASH = Digest::SHA1
439+
end
440+
RUBY
375441
assert_equal 0, cop.offenses.count
376442
end
377443
end

0 commit comments

Comments
 (0)