Merge pull request #4649 from rcorfieldffdc:defect/4646-apex-soql-injection-sobjecttype-token-variables
[apex] Add SObjectType and SObjectField to list of injectable SOQL variable types #4649
This commit is contained in:
.all-contributorsrc
docs/pages
pmd-apex/src
main/java/net/sourceforge/pmd/lang/apex/rule/security
test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml
@ -7212,6 +7212,25 @@
|
||||
"contributions": [
|
||||
"doc"
|
||||
]
|
||||
},
|
||||
{
|
||||
"login": "rcorfieldffdc",
|
||||
"name": "Richard Corfield",
|
||||
"avatar_url": "https://avatars.githubusercontent.com/u/42997936?v=4",
|
||||
"profile": "https://github.com/rcorfieldffdc",
|
||||
"contributions": [
|
||||
"code"
|
||||
]
|
||||
},
|
||||
{
|
||||
"login": "m0rjc",
|
||||
"name": "Richard Corfield",
|
||||
"avatar_url": "https://avatars.githubusercontent.com/u/994206?v=4",
|
||||
"profile": "https://github.com/m0rjc",
|
||||
"contributions": [
|
||||
"bug",
|
||||
"code"
|
||||
]
|
||||
}
|
||||
],
|
||||
"contributorsPerLine": 7,
|
||||
|
File diff suppressed because it is too large
Load Diff
@ -60,6 +60,8 @@ The remaining section describes the complete release notes for 7.0.0.
|
||||
* [#4521](https://github.com/pmd/pmd/issues/4521): \[doc] Website is not mobile friendly
|
||||
* apex-design
|
||||
* [#4596](https://github.com/pmd/pmd/issues/4596): \[apex] ExcessivePublicCount ignores properties
|
||||
* apex-security
|
||||
* [#4646](https://github.com/pmd/pmd/issues/4646): \[apex] ApexSOQLInjection does not recognise SObjectType or SObjectField as safe variable types
|
||||
* java
|
||||
* [#4401](https://github.com/pmd/pmd/issues/4401): \[java] PMD 7 fails to build under Java 19
|
||||
* java-bestpractices
|
||||
@ -228,6 +230,7 @@ The following classes have been removed:
|
||||
#### External Contributions
|
||||
* [#4528](https://github.com/pmd/pmd/pull/4528): \[apex] Update to apexlink - [Kevin Jones](https://github.com/nawforce) (@nawforce)
|
||||
* [#4637](https://github.com/pmd/pmd/pull/4637): \[java] fix #4634 - JUnit4TestShouldUseTestAnnotation false positive with TestNG - [Krystian Dabrowski](https://github.com/krdabrowski) (@krdabrowski)
|
||||
* [#4649](https://github.com/pmd/pmd/pull/4649): \[apex] Add SObjectType and SObjectField to list of injectable SOQL variable types - [Richard Corfield](https://github.com/rcorfieldffdc) (@rcorfieldffdc)
|
||||
* [#4651](https://github.com/pmd/pmd/pull/4651): \[doc] Add "Tencent Cloud Code Analysis" in Tools / Integrations - [yale](https://github.com/cyw3) (@cyw3)
|
||||
* [#4664](https://github.com/pmd/pmd/pull/4664): \[cli] CPD: Fix NPE when only `--file-list` is specified - [Wener](https://github.com/wener-tiobe) (@wener-tiobe)
|
||||
* [#4665](https://github.com/pmd/pmd/pull/4665): \[java] Doc: Fix references AutoClosable -> AutoCloseable - [Andrey Bozhko](https://github.com/AndreyBozhko) (@AndreyBozhko)
|
||||
@ -609,6 +612,8 @@ Language specific fixes:
|
||||
* [#2667](https://github.com/pmd/pmd/issues/2667): \[apex] Integrate nawforce/ApexLink to build robust Unused rule
|
||||
* [#4509](https://github.com/pmd/pmd/issues/4509): \[apex] ExcessivePublicCount doesn't consider inner classes correctly
|
||||
* [#4596](https://github.com/pmd/pmd/issues/4596): \[apex] ExcessivePublicCount ignores properties
|
||||
* apex-security
|
||||
* [#4646](https://github.com/pmd/pmd/issues/4646): \[apex] ApexSOQLInjection does not recognise SObjectType or SObjectField as safe variable types
|
||||
* java
|
||||
* [#520](https://github.com/pmd/pmd/issues/520): \[java] Allow `@SuppressWarnings` with constants instead of literals
|
||||
* [#864](https://github.com/pmd/pmd/issues/864): \[java] Similar/duplicated implementations for determining FQCN
|
||||
@ -815,6 +820,7 @@ Language specific fixes:
|
||||
* [#4542](https://github.com/pmd/pmd/pull/4542): \[java] Fix #4510: A false positive about ConstructorCallsOverridableMethod and @<!-- -->Value - [AnnaDev](https://github.com/LynnBroe) (@LynnBroe)
|
||||
* [#4553](https://github.com/pmd/pmd/pull/4553): \[java] Fix #4492: GuardLogStatement gives false positive when argument is a Java method reference - [Anastasiia Koba](https://github.com/anastasiia-koba) (@anastasiia-koba)
|
||||
* [#4637](https://github.com/pmd/pmd/pull/4637): \[java] fix #4634 - JUnit4TestShouldUseTestAnnotation false positive with TestNG - [Krystian Dabrowski](https://github.com/krdabrowski) (@krdabrowski)
|
||||
* [#4649](https://github.com/pmd/pmd/pull/4649): \[apex] Add SObjectType and SObjectField to list of injectable SOQL variable types - [Richard Corfield](https://github.com/rcorfieldffdc) (@rcorfieldffdc)
|
||||
* [#4651](https://github.com/pmd/pmd/pull/4651): \[doc] Add "Tencent Cloud Code Analysis" in Tools / Integrations - [yale](https://github.com/cyw3) (@cyw3)
|
||||
* [#4664](https://github.com/pmd/pmd/pull/4664): \[cli] CPD: Fix NPE when only `--file-list` is specified - [Wener](https://github.com/wener-tiobe) (@wener-tiobe)
|
||||
* [#4665](https://github.com/pmd/pmd/pull/4665): \[java] Doc: Fix references AutoClosable -> AutoCloseable - [Andrey Bozhko](https://github.com/AndreyBozhko) (@AndreyBozhko)
|
||||
|
@ -4,6 +4,7 @@
|
||||
|
||||
package net.sourceforge.pmd.lang.apex.rule.security;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
@ -11,6 +12,8 @@ import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Collectors;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTAssignmentExpression;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTBinaryExpression;
|
||||
@ -35,12 +38,12 @@ import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
|
||||
*
|
||||
*/
|
||||
public class ApexSOQLInjectionRule extends AbstractApexRule {
|
||||
private static final String DOUBLE = "double";
|
||||
private static final String LONG = "long";
|
||||
private static final String DECIMAL = "decimal";
|
||||
private static final String BOOLEAN = "boolean";
|
||||
private static final String ID = "id";
|
||||
private static final String INTEGER = "integer";
|
||||
private static final Set<String> SAFE_VARIABLE_TYPES =
|
||||
Collections.unmodifiableSet(Stream.of(
|
||||
"double", "long", "decimal", "boolean", "id", "integer",
|
||||
"sobjecttype", "schema.sobjecttype", "sobjectfield", "schema.sobjectfield"
|
||||
).collect(Collectors.toSet()));
|
||||
|
||||
private static final String JOIN = "join";
|
||||
private static final String ESCAPE_SINGLE_QUOTES = "escapeSingleQuotes";
|
||||
private static final String STRING = "String";
|
||||
@ -108,23 +111,16 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
|
||||
return Helper.isMethodName(m, DATABASE, QUERY) || Helper.isMethodName(m, DATABASE, COUNT_QUERY);
|
||||
}
|
||||
|
||||
private boolean isSafeVariableType(String typeName) {
|
||||
return SAFE_VARIABLE_TYPES.contains(typeName.toLowerCase(Locale.ROOT));
|
||||
}
|
||||
|
||||
private void findSafeVariablesInSignature(ASTMethod m) {
|
||||
for (ASTParameter p : m.findChildrenOfType(ASTParameter.class)) {
|
||||
switch (p.getType().toLowerCase(Locale.ROOT)) {
|
||||
case ID:
|
||||
case INTEGER:
|
||||
case BOOLEAN:
|
||||
case DECIMAL:
|
||||
case LONG:
|
||||
case DOUBLE:
|
||||
if (isSafeVariableType(p.getType())) {
|
||||
safeVariables.add(Helper.getFQVariableName(p));
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private void findSanitizedVariables(ApexNode<?> node) {
|
||||
@ -159,17 +155,8 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
|
||||
}
|
||||
|
||||
if (node instanceof ASTVariableDeclaration) {
|
||||
switch (((ASTVariableDeclaration) node).getType().toLowerCase(Locale.ROOT)) {
|
||||
case INTEGER:
|
||||
case ID:
|
||||
case BOOLEAN:
|
||||
case DECIMAL:
|
||||
case LONG:
|
||||
case DOUBLE:
|
||||
if (isSafeVariableType(((ASTVariableDeclaration) node).getType())) {
|
||||
safeVariables.add(Helper.getFQVariableName(left));
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
56
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml
56
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml
@ -62,6 +62,62 @@ public class Foo {
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>SObjectType and Field as parameters are safe to use in SOQL query string building #4646</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public with sharing class Foo {
|
||||
public void getUniqueValues(SObjectType type, SObjectField field) {
|
||||
String query = 'SELECT ' + field + ' FROM ' + type;
|
||||
Database.query(query);
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>SObjectType and Field as variables are safe to use in SOQL query string building #4646</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public with sharing class Foo {
|
||||
public void getUniqueValues() {
|
||||
SObjectType type = getSObjectType();
|
||||
SObjectField field = getSObjectField();
|
||||
String query = 'SELECT ' + field + ' FROM ' + type;
|
||||
Database.query(query);
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Schema.SObjectType and Field as parameters are safe to use in SOQL query string building #4646</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public with sharing class Foo {
|
||||
public void getUniqueValues(Schema.SObjectType type, Schema.SObjectField field) {
|
||||
String query = 'SELECT ' + field + ' FROM ' + type;
|
||||
Database.query(query);
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Schema.SObjectType and Field as variables are safe to use in SOQL query string building #4646</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public with sharing class Foo {
|
||||
public void getUniqueValues() {
|
||||
Schema.SObjectType type = getSObjectType();
|
||||
Schema.SObjectField field = getSObjectField();
|
||||
String query = 'SELECT ' + field + ' FROM ' + type;
|
||||
Database.query(query);
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Safe SOQL + merged variable from a literal</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
|
Reference in New Issue
Block a user