phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug fix to improve detection of concatenated vars

Browse Source
This commit is contained in:
Sergey
2017-01-13 14:06:08 -08:00
committed by Juan Martín Sotuyo Dodero
parent 7e83147a61
commit fb25329e0d
2 changed files with 34 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSOQLInjectionRule.java
View File

@@ -33,9 +33,9 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
private static final String STRING = "String";
private static final String DATABASE = "Database";
private static final String QUERY = "query";
private static final Pattern SELECT_PATTERN = Pattern.compile("^select[\\s]+?.*?$", Pattern.CASE_INSENSITIVE);
private final HashSet<String> safeVariables = new HashSet<>();
private final HashMap<String, Boolean> selectContainingVariables = new HashMap<>();
private static final Pattern pattern = Pattern.compile("^select[\\s]+?.+?$", Pattern.CASE_INSENSITIVE);
public ApexSOQLInjectionRule() {
setProperty(CODECLIMATE_CATEGORIES, new String[] { "Security" });
@@ -50,13 +50,6 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
return data;
}
// baz = String.escapeSignleQuotes(...);
final List<ASTAssignmentExpression> assignmentCalls = node.findDescendantsOfType(ASTAssignmentExpression.class);
for (ASTAssignmentExpression a : assignmentCalls) {
findSanitizedVariables(a);
findSelectContainingVariables(a);
}
final List<ASTFieldDeclaration> fieldExpr = node.findDescendantsOfType(ASTFieldDeclaration.class);
for (ASTFieldDeclaration a : fieldExpr) {
findSanitizedVariables(a);
@@ -70,6 +63,13 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
findSelectContainingVariables(a);
}
// baz = String.escapeSignleQuotes(...);
final List<ASTAssignmentExpression> assignmentCalls = node.findDescendantsOfType(ASTAssignmentExpression.class);
for (ASTAssignmentExpression a : assignmentCalls) {
findSanitizedVariables(a);
findSelectContainingVariables(a);
}
// Database.query(...) check
final List<ASTMethodCallExpression> potentialDbQueryCalls = node
.findDescendantsOfType(ASTMethodCallExpression.class);
@@ -80,7 +80,7 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
reportVariables(m, data);
}
}
safeVariables.clear();
selectContainingVariables.clear();
@@ -97,7 +97,7 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
if (left != null) {
Object o = literal.getNode().getLiteral();
if (o instanceof String) {
if (pattern.matcher((String) o).matches()) {
if (SELECT_PATTERN.matcher((String) o).matches()) {
selectContainingVariables.put(Helper.getFQVariableName(left), Boolean.TRUE);
} else {
safeVariables.add(Helper.getFQVariableName(left));
@@ -144,14 +144,19 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
Object o = literal.getNode().getLiteral();
if (o instanceof String) {
if (pattern.matcher((String) o).matches()) {
if (SELECT_PATTERN.matcher((String) o).matches()) {
if (!isSafeVariable) {
// select literal + other unsafe vars
selectContainingVariables.put(Helper.getFQVariableName(var), Boolean.FALSE);
}
}
}
} else {
if (!isSafeVariable) {
selectContainingVariables.put(Helper.getFQVariableName(var), Boolean.FALSE);
}
}
}
private void reportStrings(ASTMethodCallExpression m, Object data) {

19
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml
View File

@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data>
<test-code>
<test-code>
<description>Potentially unsafe SOQL on concatenation of variables 1
</description>
<expected-problems>1</expected-problems>
@@ -186,6 +186,23 @@ public class Foo {
public void test1(String field1) {
Database.query('SELECT Id FROM Account WHERE Id =' + String.escapeSingleQuotes(field1) + 'string');
}
}
]]></code>
</test-code>
<test-code>
<description>Unsafe SOQL merged from many variables</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public List<SObject> test1(String fieldNameQuery, String objName) {
String baseQuery = 'Select ';
String finalObjectQuery;
finalObjectQuery = baseQuery + fieldNameQuery + ' from ' + objName;
return Database.query(finalObjectQuery);
}
}
]]></code>
</test-code>