Skip to content

Commit e6a6c60

Browse files
guillaume-dequenneguillaume-dequenne
authored
SONARPY-826 S2612 to handle binary OR expressions (SonarSource#894)
1 parent 3429a20 commit e6a6c60

3 files changed

Lines changed: 41 additions & 9 deletions

File tree

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88
'project:docker-compose-1.24.1/script/release/release/images.py':[
99
33,
1010
],
11+
'project:tensorflow/python/debug/cli/debugger_cli_common_test.py':[
12+
1046,
13+
],
1114
'project:tensorflow/python/lib/io/file_io_test.py':[
1215
634,
1316
],

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

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,15 @@
2020
package org.sonar.python.checks;
2121

2222
import java.util.Arrays;
23+
import java.util.HashSet;
2324
import java.util.List;
25+
import java.util.Set;
2426
import org.sonar.check.Rule;
2527
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2628
import org.sonar.plugins.python.api.SubscriptionContext;
2729
import org.sonar.plugins.python.api.symbols.Symbol;
2830
import org.sonar.plugins.python.api.tree.Argument;
31+
import org.sonar.plugins.python.api.tree.BinaryExpression;
2932
import org.sonar.plugins.python.api.tree.CallExpression;
3033
import org.sonar.plugins.python.api.tree.Expression;
3134
import org.sonar.plugins.python.api.tree.HasSymbol;
@@ -75,21 +78,38 @@ private static void checkSensitiveArgument(List<Argument> arguments , int sensit
7578
return;
7679
}
7780
Expression expression = modeArgument.expression();
81+
if(isUnsafeExpression(expression, safeModulo, new HashSet<>())) {
82+
ctx.addIssue(modeArgument, MESSAGE);
83+
}
84+
}
85+
86+
private static boolean isUnsafeExpression(Expression expression, int safeModulo, Set<Expression> checkedExpressions) {
87+
if (checkedExpressions.contains(expression)) {
88+
return false;
89+
}
90+
checkedExpressions.add(expression);
7891
if (expression instanceof HasSymbol) {
7992
Symbol symbol = ((HasSymbol) expression).symbol();
8093
if (symbol != null && SENSITIVE_CONSTANTS.contains(symbol.fullyQualifiedName())) {
81-
ctx.addIssue(modeArgument, MESSAGE);
82-
return;
94+
return true;
8395
}
8496
}
85-
if (expression.is(Tree.Kind.NAME)) {
86-
expression = Expressions.singleAssignedValue(((Name) expression));
97+
if (expression.is(Tree.Kind.BITWISE_OR)) {
98+
BinaryExpression binaryExpression = (BinaryExpression) expression;
99+
return isUnsafeExpression(binaryExpression.leftOperand(), safeModulo, checkedExpressions)
100+
|| isUnsafeExpression(binaryExpression.rightOperand(), safeModulo, checkedExpressions);
87101
}
88-
if (expression != null && expression.is(Tree.Kind.NUMERIC_LITERAL)) {
102+
if (expression.is(Tree.Kind.NUMERIC_LITERAL)) {
89103
NumericLiteral numericLiteral = (NumericLiteral) expression;
90-
if (numericLiteral.valueAsLong() % 8 != safeModulo) {
91-
ctx.addIssue(modeArgument, MESSAGE);
104+
return numericLiteral.valueAsLong() % 8 != safeModulo;
105+
}
106+
if (expression.is(Tree.Kind.NAME)) {
107+
Expression singleAssignedValue = Expressions.singleAssignedValue(((Name) expression));
108+
if (singleAssignedValue == null) {
109+
return false;
92110
}
111+
return isUnsafeExpression(singleAssignedValue, safeModulo, checkedExpressions);
93112
}
113+
return false;
94114
}
95115
}

‎python-checks/src/test/resources/checks/hotspots/filePermissions.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,14 @@
8181
os.chmod("/tmp/fs", stat.S_IWOTH) # Noncompliant
8282
os.chmod("/tmp/fs", stat.S_IXOTH) # Noncompliant
8383
os.chmod("/tmp/fs", stat.S_IRWXO) # Noncompliant
84-
os.chmod("/tmp/fs", stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) # FN
85-
os.chmod("/tmp/fs", stat.S_IROTH | stat.S_IRGRP | stat.S_IRUSR) # FN
84+
os.chmod("/tmp/fs", stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) # Noncompliant {{Make sure this permission is safe.}}
85+
os.chmod("/tmp/fs", stat.S_IROTH | stat.S_IRGRP | stat.S_IRUSR) # Noncompliant {{Make sure this permission is safe.}}
86+
x = stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO
87+
os.chmod("/tmp/fs", x) # Noncompliant
88+
y = stat.S_IRWXO
89+
os.chmod("/tmp/fs", y) # Noncompliant
90+
91+
def no_soe():
92+
some_val = other_val
93+
other_val = some_val
94+
os.chmod("/tmp/fs", other_val) # OK

0 commit comments

Comments
 (0)