Skip to content

Commit 4927ad6

Browse files
guillaume-dequenneguillaume-dequenne
authored
SONARPY-491 Rule S2053 Hashes should include an unpredictable salt (SonarSource#543)
1 parent 364de76 commit 4927ad6

8 files changed

Lines changed: 349 additions & 0 deletions

File tree

‎its/ruling/src/test/resources/expected/python-S2053.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
'project:twisted-12.1.0/twisted/conch/test/test_checkers.py':[
3+
64,
4+
78,
5+
92,
6+
519,
7+
],
8+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ public static Iterable<Class> getChecks() {
115115
OsExecCheck.class,
116116
OverwrittenCollectionEntryCheck.class,
117117
ParsingErrorCheck.class,
118+
PredictableSaltCheck.class,
118119
PreIncrementDecrementCheck.class,
119120
PrintStatementUsageCheck.class,
120121
ProcessSignallingCheck.class,

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

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks;
21+
22+
import java.util.Collections;
23+
import java.util.HashMap;
24+
import java.util.Map;
25+
import org.sonar.check.Rule;
26+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
27+
import org.sonar.plugins.python.api.SubscriptionContext;
28+
import org.sonar.plugins.python.api.symbols.Symbol;
29+
import org.sonar.plugins.python.api.tree.Argument;
30+
import org.sonar.plugins.python.api.tree.AssignmentStatement;
31+
import org.sonar.plugins.python.api.tree.CallExpression;
32+
import org.sonar.plugins.python.api.tree.Expression;
33+
import org.sonar.plugins.python.api.tree.Name;
34+
import org.sonar.plugins.python.api.tree.RegularArgument;
35+
import org.sonar.plugins.python.api.tree.Tree;
36+
import org.sonar.python.tree.TreeUtils;
37+
38+
@Rule(key = "S2053")
39+
public class PredictableSaltCheck extends PythonSubscriptionCheck {
40+
41+
private static final String MISSING_SALT_MESSAGE = "Add an unpredictable salt value to this hash.";
42+
private static final String PREDICTABLE_SALT_MESSAGE = "Make this salt unpredictable.";
43+
private Map<String, Integer> sensitiveArgumentByFQN;
44+
45+
@Override
46+
public void initialize(Context context) {
47+
context.registerSyntaxNodeConsumer(Tree.Kind.CALL_EXPR, ctx -> handleCallExpression((CallExpression) ctx.syntaxNode(), ctx));
48+
}
49+
50+
private void handleCallExpression(CallExpression callExpression, SubscriptionContext ctx) {
51+
Symbol calleeSymbol = callExpression.calleeSymbol();
52+
if (calleeSymbol == null) {
53+
return;
54+
}
55+
if (sensitiveArgumentByFQN().containsKey(calleeSymbol.fullyQualifiedName())) {
56+
int argNb = sensitiveArgumentByFQN().get(calleeSymbol.fullyQualifiedName());
57+
checkArguments(callExpression, argNb, ctx);
58+
}
59+
}
60+
61+
private static void checkArguments(CallExpression callExpression, int argNb, SubscriptionContext ctx) {
62+
if (callExpression.arguments().size() <= argNb) {
63+
ctx.addIssue(callExpression.callee(), MISSING_SALT_MESSAGE);
64+
}
65+
for (int i = 0; i < callExpression.arguments().size(); i++) {
66+
Argument argument = callExpression.arguments().get(i);
67+
if (argument.is(Tree.Kind.REGULAR_ARGUMENT)) {
68+
RegularArgument regularArgument = (RegularArgument) argument;
69+
Name keywordArgument = regularArgument.keywordArgument();
70+
if (keywordArgument != null) {
71+
if (keywordArgument.name().equals("salt")) {
72+
checkSensitiveArgument(regularArgument, ctx);
73+
}
74+
} else if (i == argNb) {
75+
checkSensitiveArgument(regularArgument, ctx);
76+
}
77+
}
78+
}
79+
}
80+
81+
private static void checkSensitiveArgument(RegularArgument regularArgument, SubscriptionContext ctx) {
82+
if (regularArgument.expression().is(Tree.Kind.NAME)) {
83+
Expression expression = Expressions.singleAssignedValue((Name) regularArgument.expression());
84+
if (expression == null) {
85+
return;
86+
}
87+
if (expression.is(Tree.Kind.STRING_LITERAL)) {
88+
AssignmentStatement assignmentStatement = (AssignmentStatement) TreeUtils.firstAncestorOfKind(expression, Tree.Kind.ASSIGNMENT_STMT);
89+
ctx.addIssue(regularArgument, PREDICTABLE_SALT_MESSAGE).secondary(assignmentStatement, null);
90+
}
91+
}
92+
if (regularArgument.expression().is(Tree.Kind.STRING_LITERAL)) {
93+
ctx.addIssue(regularArgument, PREDICTABLE_SALT_MESSAGE);
94+
}
95+
}
96+
97+
private Map<String, Integer> sensitiveArgumentByFQN() {
98+
if (sensitiveArgumentByFQN == null) {
99+
sensitiveArgumentByFQN = new HashMap<>();
100+
sensitiveArgumentByFQN.put("hashlib.pbkdf2_hmac", 2);
101+
sensitiveArgumentByFQN.put("hashlib.scrypt", 4);
102+
sensitiveArgumentByFQN.put("crypt.crypt", 1);
103+
sensitiveArgumentByFQN.put("cryptography.hazmat.primitives.kdf.pbkdf2.PBKDF2HMAC", 2);
104+
sensitiveArgumentByFQN.put("Cryptodome.Protocol.KDF.PBKDF2", 1);
105+
sensitiveArgumentByFQN.put("Cryptodome.Protocol.KDF.scrypt", 1);
106+
sensitiveArgumentByFQN.put("Cryptodome.Protocol.KDF.bcrypt", 2);
107+
sensitiveArgumentByFQN = Collections.unmodifiableMap(sensitiveArgumentByFQN);
108+
}
109+
return sensitiveArgumentByFQN;
110+
}
111+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S2053.html‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<p>In cryptography, "salt" is extra piece of data which is included in a hashing algorithm. It makes dictionary attacks more difficult. Using a
2+
cryptographic hash function without an unpredictable salt increases the likelihood that an attacker will be able to successfully guess a hashed value
3+
such as a password with a dictionary attack.</p>
4+
<p>This rule raises an issue when a hashing function which has been specifically designed for hashing sensitive data, such as pbkdf2, is used with a
5+
non-random, reused or too short salt value. It does not raise an issue on base hashing algorithms such as sha1 or md5 as these are often used for
6+
other purposes.</p>
7+
<h2>Recommended Secure Coding Practices</h2>
8+
<ul>
9+
<li> use hashing functions generating their own salt or generate a long random salt of at least 32 bytes. </li>
10+
<li> the salt is at least as long as the resulting hash value. </li>
11+
<li> provide the salt to a safe hashing function such as PBKDF2. </li>
12+
<li> save both the salt and the hashed value in the relevant database record; during future validation operations, the salt and hash can then be
13+
retrieved from the database. The hash is recalculated with the stored salt and the value being validated, and the result compared to the stored
14+
hash. </li>
15+
</ul>
16+
<h2>Noncompliant Code Example</h2>
17+
<p>hashlib</p>
18+
<pre>
19+
import crypt
20+
from hashlib import pbkdf2_hmac
21+
22+
hash = pbkdf2_hmac('sha256', password, b'D8VxSmTZt2E2YV454mkqAY5e', 100000) # Noncompliant: salt is hardcoded
23+
</pre>
24+
<p>crypt</p>
25+
<pre>
26+
hash = crypt.crypt(password) # Noncompliant: salt is not provided
27+
</pre>
28+
<h2>Compliant Solution</h2>
29+
<p>hashlib</p>
30+
<pre>
31+
import crypt
32+
from hashlib import pbkdf2_hmac
33+
34+
salt = os.urandom(32)
35+
hash = pbkdf2_hmac('sha256', password, salt, 100000) # Compliant
36+
</pre>
37+
<p>crypt</p>
38+
<pre>
39+
salt = crypt.mksalt(crypt.METHOD_SHA256)
40+
hash = crypt.crypt(password, salt) # Compliant
41+
</pre>
42+
<h2>See</h2>
43+
<ul>
44+
<li> <a href="https://www.owasp.org/index.php/Top_10-2017_A3-Sensitive_Data_Exposure">OWASP Top 10 2017 Category A3</a> - Sensitive Data Exposure
45+
</li>
46+
<li> <a href="http://cwe.mitre.org/data/definitions/759.html">MITRE, CWE-759</a> - Use of a One-Way Hash without a Salt </li>
47+
<li> <a href="http://cwe.mitre.org/data/definitions/760.html">MITRE, CWE-760</a> - Use of a One-Way Hash with a Predictable Salt </li>
48+
<li> <a href="https://www.sans.org/top25-software-errors/#cat3">SANS Top 25</a> - Porous Defenses </li>
49+
</ul>
50+

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/S2053.json‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
{
2+
"title": "Hashes should include an unpredictable salt",
3+
"type": "VULNERABILITY",
4+
"status": "ready",
5+
"remediation": {
6+
"func": "Constant\/Issue",
7+
"constantCost": "30min"
8+
},
9+
"tags": [
10+
"cwe",
11+
"sans-top25-porous",
12+
"owasp-a3"
13+
],
14+
"defaultSeverity": "Critical",
15+
"ruleSpecification": "RSPEC-2053",
16+
"sqKey": "S2053",
17+
"scope": "All",
18+
"securityStandards": {
19+
"CWE": [
20+
759,
21+
760
22+
],
23+
"OWASP": [
24+
"A3"
25+
]
26+
}
27+
}

‎python-checks/src/main/resources/org/sonar/l10n/py/rules/python/Sonar_way_profile.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
"S1862",
3636
"S1871",
3737
"S2068",
38+
"S2053",
3839
"S2077",
3940
"S2190",
4041
"S2245",

‎python-checks/src/test/java/org/sonar/python/checks/PredictableSaltCheckTest.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* SonarQube Python Plugin
3+
* Copyright (C) 2011-2020 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
package org.sonar.python.checks;
21+
22+
import org.junit.Test;
23+
import org.sonar.python.checks.utils.PythonCheckVerifier;
24+
25+
public class PredictableSaltCheckTest {
26+
27+
@Test
28+
public void test() {
29+
PythonCheckVerifier.verify("src/test/resources/checks/predictableSalt.py", new PredictableSaltCheck());
30+
}
31+
32+
}

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

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import crypt
2+
import base64
3+
import os
4+
import hashlib
5+
import os
6+
7+
# crypt-salt
8+
password = 'password123'
9+
salt = crypt.mksalt(crypt.METHOD_SHA256)
10+
11+
hash = crypt.crypt(password) # Noncompliant {{Add an unpredictable salt value to this hash.}}
12+
# ^^^^^^^^^^^
13+
hash = crypt.crypt(password, "") # Noncompliant {{Make this salt unpredictable.}}
14+
# ^^
15+
hash = crypt.crypt(password, salt) # Compliant
16+
17+
18+
19+
# hashlib-salt
20+
email = 'info@sonarsource'
21+
password = 'password123'
22+
salt = os.urandom(32)
23+
24+
hash = hashlib.pbkdf2_hmac('sha256', password, b'', 100000) # Noncompliant {{Make this salt unpredictable.}}
25+
hash = hashlib.pbkdf2_hmac('sha256', password, b'D8VxSmTZt2E2YV454mkqAY5e', 100000) # Noncompliant {{Make this salt unpredictable.}}
26+
hash = hashlib.pbkdf2_hmac('sha256', password, email, 100000) # Noncompliant {{Make this salt unpredictable.}}
27+
hash = hashlib.scrypt(password, n=1024, r=1, p=1, salt=b'') # Noncompliant {{Make this salt unpredictable.}}
28+
# ^^^^^^^^
29+
30+
hash_ = hashlib.pbkdf2_hmac('sha256', password.encode('utf-8'), salt, 100000) # Compliant
31+
hash_ = hashlib.pbkdf2_hmac('sha256', password.encode('utf-8'), unknown, 100000) # Compliant
32+
hash_ = hashlib.pbkdf2_hmac('sha256', password.encode('utf-8'), *unpack, 100000) # Compliant
33+
def func():
34+
salt = "string_literal"
35+
# ^^^^^^^^^^^^^^^^^^^^^^^>
36+
hash = hashlib.pbkdf2_hmac('sha256', password, salt, 100000) # Noncompliant {{Make this salt unpredictable.}}
37+
# ^^^^
38+
39+
# cryptography-salt
40+
from cryptography.hazmat.primitives import hashes
41+
from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC
42+
from cryptography.hazmat.backends import default_backend
43+
44+
def derive_password(password, salt, backend):
45+
kdf = PBKDF2HMAC(
46+
algorithm=hashes.SHA256(),
47+
length=32,
48+
salt=b'D8VxSmTZt2E2YV454mkqAY5e', # Noncompliant {{Make this salt unpredictable.}}
49+
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
50+
iterations=100000,
51+
backend=backend
52+
)
53+
key = kdf.derive(password)
54+
55+
kdf = PBKDF2HMAC(
56+
algorithm=hashes.SHA256(),
57+
length=32,
58+
salt=salt, # Compliant
59+
iterations=100000,
60+
backend=backend
61+
)
62+
key = kdf.derive(password)
63+
64+
salt_ = os.urandom(16)
65+
kdf = PBKDF2HMAC(
66+
algorithm=hashes.SHA256(),
67+
length=32,
68+
salt=salt_, # Compliant
69+
iterations=100000,
70+
backend=backend
71+
)
72+
key = kdf.derive(password)
73+
74+
salt = os.urandom(16)
75+
backend = default_backend()
76+
derive_password(b"my great password", salt, backend)
77+
78+
79+
#cryptodome-salt
80+
81+
from Cryptodome.Protocol.KDF import PBKDF2, scrypt, bcrypt
82+
from Crypto.Hash import SHA512
83+
from Crypto.Random import get_random_bytes
84+
85+
86+
def derive_password(password, salt):
87+
88+
PBKDF2(password,
89+
b'D8VxSmTZt2E2YV454mkqAY5e', # Noncompliant {{Make this salt unpredictable.}}
90+
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^
91+
32, count=100000
92+
)
93+
94+
key = scrypt(password, b'D8VxSmTZt2E2YV454mkqAY5e', 32, N=2**14, r=8, p=1) # Noncompliant {{Make this salt unpredictable.}}
95+
# ^^^^^^^^^^^^^^^^^^^^^^^^^^^
96+
key = bcrypt(password, 12, b'D8VxSmTZt2E2YV45') # Noncompliant {{Make this salt unpredictable.}}
97+
# ^^^^^^^^^^^^^^^^^^^
98+
99+
100+
PBKDF2(password, salt, # Compliant
101+
32, count=100000
102+
)
103+
104+
salt_ = get_random_bytes(32)
105+
PBKDF2(password, salt_, # Compliant
106+
32, count=100000,
107+
)
108+
109+
key = scrypt(password, salt_, 32, N=2**14, r=8, p=1) # Compliant
110+
111+
salt_16 = get_random_bytes(16)
112+
key = bcrypt(password, 12, salt_16) # Compliant
113+
114+
115+
salt = get_random_bytes(32)
116+
password = b'my super secret'
117+
derive_password(password, salt)
118+
119+
unknown.openUnknown(open(__file__).read())

0 commit comments

Comments
 (0)