Skip to content

Commit 9a6747e

Browse files
andrey-tyukin-sonarsourceandrey-tyukin-sonarsource
authored
SONARPY-469 Rule 5547: standardize issue messages (SonarSource#700)
1 parent 6681c18 commit 9a6747e

2 files changed

Lines changed: 32 additions & 63 deletions

File tree

‎python-checks/src/main/java/org/sonar/python/checks/RobustCipherAlgorithmCheck.java‎

Lines changed: 15 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@
1919
*/
2020
package org.sonar.python.checks;
2121

22-
import java.util.AbstractMap;
23-
import java.util.HashMap;
24-
import java.util.Map;
22+
import java.util.HashSet;
2523
import static java.util.Arrays.asList;
2624

2725
import org.sonar.check.Rule;
@@ -35,59 +33,30 @@
3533
@Rule(key = "S5547")
3634
public class RobustCipherAlgorithmCheck extends PythonSubscriptionCheck {
3735

38-
private static final Map<String, String> sensitiveCalleeFqnsAndMessages = new HashMap<>();
36+
private static final String MESSAGE = "Use a strong cipher algorithm.";
37+
private static final HashSet<String> sensitiveCalleeFqns = new HashSet<>();
3938

4039
static {
41-
String desMessage = "DES works with 56-bit keys that allow attacks via exhaustive search.";
42-
String des3Message = "Triple DES is vulnerable to meet-in-the-middle attacks.";
43-
String rc2Message = "RC2 is vulnerable to a related-key attack.";
44-
String rc4Message = "RC4 is vulnerable to several attacks.";
45-
String blowfishMessage = "Blowfish uses a 64-bit block size, which makes it vulnerable to birthday attacks.";
46-
47-
// Idea is listed under "Weak Algorithms" in pyca/cryptography documentation
48-
// https://cryptography.io/en/latest/hazmat/primitives/symmetric-encryption/\
49-
// #cryptography.hazmat.primitives.ciphers.algorithms.IDEA
50-
String ideaMessage = "IDEA-cipher is susceptible to attacks when using weak keys.";
51-
5240
// `pycryptodomex`, `pycryptodome`, and `pycrypto` all share the same names of the algorithms,
5341
// moreover, `pycryptodome` is drop-in replacement for `pycrypto`, thus they share same name ("Crypto").
5442
for (String libraryName : asList("Cryptodome", "Crypto")) {
55-
for (Map.Entry<String, String> e : asList(
56-
entry("DES", desMessage),
57-
entry("DES3", des3Message),
58-
entry("ARC2", rc2Message),
59-
entry("ARC4", rc4Message),
60-
entry("Blowfish", blowfishMessage))) {
61-
62-
String methodName = e.getKey();
63-
String message = e.getValue();
64-
sensitiveCalleeFqnsAndMessages.put(String.format("%s.Cipher.%s.new", libraryName, methodName), message);
43+
for (String vulnerableMethodName : asList("DES", "DES3", "ARC2", "ARC4", "Blowfish")) {
44+
sensitiveCalleeFqns.add(String.format("%s.Cipher.%s.new", libraryName, vulnerableMethodName));
6545
}
6646
}
6747

68-
// pyca (pyca/cryptography)
69-
for (Map.Entry<String, String> e : asList(
70-
entry("TripleDES", des3Message),
71-
entry("Blowfish", blowfishMessage),
72-
entry("ARC4", rc4Message),
73-
entry("IDEA", ideaMessage))) {
7448

75-
String methodName = e.getKey();
76-
String message = e.getValue();
77-
sensitiveCalleeFqnsAndMessages.put(
78-
String.format("cryptography.hazmat.primitives.ciphers.algorithms.%s", methodName),
79-
message);
49+
// Idea is listed under "Weak Algorithms" in pyca/cryptography documentation
50+
// https://cryptography.io/en/latest/hazmat/primitives/symmetric-encryption/\
51+
// #cryptography.hazmat.primitives.ciphers.algorithms.IDEA
52+
// pyca (pyca/cryptography)
53+
for (String methodName : asList("TripleDES", "Blowfish", "ARC4", "IDEA")) {
54+
sensitiveCalleeFqns.add(String.format("cryptography.hazmat.primitives.ciphers.algorithms.%s", methodName));
8055
}
8156

8257
// pydes
83-
sensitiveCalleeFqnsAndMessages.put("pyDes.des", desMessage);
84-
sensitiveCalleeFqnsAndMessages.put("pyDes.triple_des", des3Message);
85-
}
86-
87-
/** Pair constructor. */
88-
private static <K, V> Map.Entry<K, V> entry(K key, V value) {
89-
// It's not uncommon: https://www.baeldung.com/java-initialize-hashmap#the-java-8-way
90-
return new AbstractMap.SimpleEntry<>(key, value);
58+
sensitiveCalleeFqns.add("pyDes.des");
59+
sensitiveCalleeFqns.add("pyDes.triple_des");
9160
}
9261

9362
@Override
@@ -97,8 +66,8 @@ public void initialize(Context context) {
9766
Symbol calleeSymbol = callExpr.calleeSymbol();
9867
if (calleeSymbol != null) {
9968
String fqn = calleeSymbol.fullyQualifiedName();
100-
if (fqn != null && sensitiveCalleeFqnsAndMessages.containsKey(fqn)) {
101-
subscriptionContext.addIssue(callExpr.callee(), sensitiveCalleeFqnsAndMessages.get(fqn));
69+
if (fqn != null && sensitiveCalleeFqns.contains(fqn)) {
70+
subscriptionContext.addIssue(callExpr.callee(), MESSAGE);
10271
}
10372
}
10473
});

‎python-checks/src/test/resources/checks/robustCipherAlgorithm.py‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,23 @@ def pycryptodomexExamples():
33
from Cryptodome.Random import get_random_bytes
44

55
key = b'-8B key-'
6-
DES.new(key, DES.MODE_OFB) # Noncompliant {{DES works with 56-bit keys that allow attacks via exhaustive search.}}
6+
DES.new(key, DES.MODE_OFB) # Noncompliant {{Use a strong cipher algorithm.}}
77
# ^^^^^^^
88

99
key = DES3.adjust_key_parity(get_random_bytes(24))
10-
cipher = DES3.new(key, DES3.MODE_CFB) # Noncompliant {{Triple DES is vulnerable to meet-in-the-middle attacks.}}
10+
cipher = DES3.new(key, DES3.MODE_CFB) # Noncompliant {{Use a strong cipher algorithm.}}
1111
# ^^^^^^^^
1212

1313
key = b'Sixteen byte key'
14-
cipher = ARC2.new(key, ARC2.MODE_CFB) # Noncompliant {{RC2 is vulnerable to a related-key attack.}}
14+
cipher = ARC2.new(key, ARC2.MODE_CFB) # Noncompliant {{Use a strong cipher algorithm.}}
1515
# ^^^^^^^^
1616

1717
key = b'Very long and confidential key'
18-
cipher = ARC4.new(key) # Noncompliant {{RC4 is vulnerable to several attacks.}}
18+
cipher = ARC4.new(key) # Noncompliant {{Use a strong cipher algorithm.}}
1919
# ^^^^^^^^
2020

2121
key = b'An arbitrarily long key'
22-
cipher = Blowfish.new(key, Blowfish.MODE_CBC) # Noncompliant {{Blowfish uses a 64-bit block size, which makes it vulnerable to birthday attacks.}}
22+
cipher = Blowfish.new(key, Blowfish.MODE_CBC) # Noncompliant {{Use a strong cipher algorithm.}}
2323
# ^^^^^^^^^^^^
2424

2525
key = b'Sixteen byte key'
@@ -38,20 +38,20 @@ def pycroptodomeExamples():
3838
from Crypto.Random import get_random_bytes
3939

4040
key = b'-8B key-'
41-
DES.new(key, DES.MODE_OFB) # Noncompliant {{DES works with 56-bit keys that allow attacks via exhaustive search.}}
41+
DES.new(key, DES.MODE_OFB) # Noncompliant {{Use a strong cipher algorithm.}}
4242
# ^^^^^^^
4343

4444
key = DES3.adjust_key_parity(get_random_bytes(24))
45-
cipher = DES3.new(key, DES3.MODE_CFB) # Noncompliant {{Triple DES is vulnerable to meet-in-the-middle attacks.}}
45+
cipher = DES3.new(key, DES3.MODE_CFB) # Noncompliant {{Use a strong cipher algorithm.}}
4646
# ^^^^^^^^
4747
key = b'Sixteen byte key'
48-
cipher = ARC2.new(key, ARC2.MODE_CFB) # Noncompliant {{RC2 is vulnerable to a related-key attack.}}
48+
cipher = ARC2.new(key, ARC2.MODE_CFB) # Noncompliant {{Use a strong cipher algorithm.}}
4949
# ^^^^^^^^
5050
key = b'Very long and confidential key'
51-
cipher = ARC4.new(key) # Noncompliant {{RC4 is vulnerable to several attacks.}}
51+
cipher = ARC4.new(key) # Noncompliant {{Use a strong cipher algorithm.}}
5252
# ^^^^^^^^
5353
key = b'An arbitrarily long key'
54-
cipher = Blowfish.new(key, Blowfish.MODE_CBC) # Noncompliant {{Blowfish uses a 64-bit block size, which makes it vulnerable to birthday attacks.}}
54+
cipher = Blowfish.new(key, Blowfish.MODE_CBC) # Noncompliant {{Use a strong cipher algorithm.}}
5555
# ^^^^^^^^^^^^
5656

5757
def pycaExamples():
@@ -62,23 +62,23 @@ def pycaExamples():
6262
key = os.urandom(16)
6363
iv = os.urandom(16)
6464

65-
tdes4 = Cipher(algorithms.TripleDES(key), mode=None, backend=default_backend()) # Noncompliant {{Triple DES is vulnerable to meet-in-the-middle attacks.}}
65+
tdes4 = Cipher(algorithms.TripleDES(key), mode=None, backend=default_backend()) # Noncompliant {{Use a strong cipher algorithm.}}
6666
# ^^^^^^^^^^^^^^^^^^^^
67-
bf3 = Cipher(algorithms.Blowfish(key), mode=None, backend=default_backend()) # Noncompliant {{Blowfish uses a 64-bit block size, which makes it vulnerable to birthday attacks.}}
67+
bf3 = Cipher(algorithms.Blowfish(key), mode=None, backend=default_backend()) # Noncompliant {{Use a strong cipher algorithm.}}
6868
# ^^^^^^^^^^^^^^^^^^^
69-
rc42 = Cipher(algorithms.ARC4(key), mode=None, backend=default_backend()) # Noncompliant {{RC4 is vulnerable to several attacks.}}
69+
rc42 = Cipher(algorithms.ARC4(key), mode=None, backend=default_backend()) # Noncompliant {{Use a strong cipher algorithm.}}
7070
# ^^^^^^^^^^^^^^^
7171

7272
def pydesExamples():
7373
import pyDes;
7474

75-
des1 = pyDes.des('ChangeIt') # Noncompliant {{DES works with 56-bit keys that allow attacks via exhaustive search.}}
75+
des1 = pyDes.des('ChangeIt') # Noncompliant {{Use a strong cipher algorithm.}}
7676
# ^^^^^^^^^
77-
des2 = pyDes.des('ChangeIt', pyDes.CBC, "\0\0\0\0\0\0\0\0", pad=None, padmode=pyDes.PAD_PKCS5) # Noncompliant {{DES works with 56-bit keys that allow attacks via exhaustive search.}}
77+
des2 = pyDes.des('ChangeIt', pyDes.CBC, "\0\0\0\0\0\0\0\0", pad=None, padmode=pyDes.PAD_PKCS5) # Noncompliant {{Use a strong cipher algorithm.}}
7878
# ^^^^^^^^^
79-
tdes1 = pyDes.triple_des('ChangeItWithYourKey!!!!!') # Noncompliant {{Triple DES is vulnerable to meet-in-the-middle attacks.}}
79+
tdes1 = pyDes.triple_des('ChangeItWithYourKey!!!!!') # Noncompliant {{Use a strong cipher algorithm.}}
8080
# ^^^^^^^^^^^^^^^^
81-
tdes2 = pyDes.triple_des('ChangeItWithYourKey!!!!!', pyDes.CBC, "\0\0\0\0\0\0\0\0", pad=None, padmode=pyDes.PAD_PKCS5) # Noncompliant {{Triple DES is vulnerable to meet-in-the-middle attacks.}}
81+
tdes2 = pyDes.triple_des('ChangeItWithYourKey!!!!!', pyDes.CBC, "\0\0\0\0\0\0\0\0", pad=None, padmode=pyDes.PAD_PKCS5) # Noncompliant {{Use a strong cipher algorithm.}}
8282
# ^^^^^^^^^^^^^^^^
8383

8484
def pycryptodomeCompliant():

0 commit comments

Comments
 (0)