From f02ec3ec55ad22082e4c826c725625f2ff06d8df Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 12 Dec 2016 09:50:45 -0800 Subject: [PATCH 1/3] Rule check-in --- .../security/ApexDangerousMethodsRule.java | 47 +++++++++++++++++++ .../main/resources/rulesets/apex/ruleset.xml | 13 ++++- .../main/resources/rulesets/apex/security.xml | 19 ++++++++ .../apex/rule/security/SecurityRulesTest.java | 1 + .../security/xml/ApexDangerousMethods.xml | 29 ++++++++++++ 5 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java create mode 100644 pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexDangerousMethods.xml diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java new file mode 100644 index 0000000000..288840bd08 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java @@ -0,0 +1,47 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.security; + +import java.util.List; + +import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; + +/** + * Flags dangerous method calls, e.g. FinancialForce + * Configuration.disableTriggerCRUDSecurity() + * + * + * @author sergey.gorbaty + * + */ +public class ApexDangerousMethodsRule extends AbstractApexRule { + private static final String DISABLE_CRUD = "disableTriggerCRUDSecurity"; + private static final String CONFIGURATION = "Configuration"; + + public ApexDangerousMethodsRule() { + setProperty(CODECLIMATE_CATEGORIES, new String[] { "Security" }); + setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); + setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); + + } + + public Object visit(ASTUserClass node, Object data) { + if (Helper.isTestMethodOrClass(node)) { + return data; + } + + List methodCalls = node.findDescendantsOfType(ASTMethodCallExpression.class); + for (ASTMethodCallExpression methodCall : methodCalls) { + if (Helper.isMethodName(methodCall, CONFIGURATION, DISABLE_CRUD)) { + addViolation(data, methodCall); + } + } + + return data; + } + +} diff --git a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml index 89aa421e8e..ef7166afd9 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/ruleset.xml @@ -261,5 +261,14 @@ - - + + + 3 + + + + + + + + \ No newline at end of file diff --git a/pmd-apex/src/main/resources/rulesets/apex/security.xml b/pmd-apex/src/main/resources/rulesets/apex/security.xml index 79aba4d396..1d1e7af75a 100644 --- a/pmd-apex/src/main/resources/rulesets/apex/security.xml +++ b/pmd-apex/src/main/resources/rulesets/apex/security.xml @@ -189,4 +189,23 @@ public class Foo { + + + Calling potentially dangerous method + + 3 + + + + + diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java index a4eebbf934..79c15cc509 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/SecurityRulesTest.java @@ -21,5 +21,6 @@ public class SecurityRulesTest extends SimpleAggregatorTst { addRule(RULESET, "ApexSharingViolations"); addRule(RULESET, "ApexInsecureEndpoint"); addRule(RULESET, "ApexCRUDViolation"); + addRule(RULESET, "ApexDangerousMethods"); } } diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexDangerousMethods.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexDangerousMethods.xml new file mode 100644 index 0000000000..d50a6ce071 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexDangerousMethods.xml @@ -0,0 +1,29 @@ + + + + + + Apex dangerous FileForce method + 1 + + + + + Apex non FileForce method + 0 + + + + From 9ffe650a964c05bb89ebf55c679751687b8f1445 Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 12 Dec 2016 13:24:44 -0800 Subject: [PATCH 2/3] Code style changes --- .../security/ApexDangerousMethodsRule.java | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java index 288840bd08..649a66418b 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java @@ -19,29 +19,29 @@ import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; * */ public class ApexDangerousMethodsRule extends AbstractApexRule { - private static final String DISABLE_CRUD = "disableTriggerCRUDSecurity"; - private static final String CONFIGURATION = "Configuration"; + private static final String DISABLE_CRUD = "disableTriggerCRUDSecurity"; + private static final String CONFIGURATION = "Configuration"; - public ApexDangerousMethodsRule() { - setProperty(CODECLIMATE_CATEGORIES, new String[] { "Security" }); - setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); - setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); + public ApexDangerousMethodsRule() { + setProperty(CODECLIMATE_CATEGORIES, new String[] { "Security" }); + setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); + setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); - } + } - public Object visit(ASTUserClass node, Object data) { - if (Helper.isTestMethodOrClass(node)) { - return data; - } + public Object visit(ASTUserClass node, Object data) { + if (Helper.isTestMethodOrClass(node)) { + return data; + } - List methodCalls = node.findDescendantsOfType(ASTMethodCallExpression.class); - for (ASTMethodCallExpression methodCall : methodCalls) { - if (Helper.isMethodName(methodCall, CONFIGURATION, DISABLE_CRUD)) { - addViolation(data, methodCall); - } - } + List methodCalls = node.findDescendantsOfType(ASTMethodCallExpression.class); + for (ASTMethodCallExpression methodCall : methodCalls) { + if (Helper.isMethodName(methodCall, CONFIGURATION, DISABLE_CRUD)) { + addViolation(data, methodCall); + } + } - return data; - } + return data; + } } From 5417c88889eb37a2a617f19085e1378f6f0d894a Mon Sep 17 00:00:00 2001 From: Sergey Date: Mon, 12 Dec 2016 13:30:50 -0800 Subject: [PATCH 3/3] Adding to the rule chain --- .../pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java index 649a66418b..dd55319cec 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java @@ -23,6 +23,7 @@ public class ApexDangerousMethodsRule extends AbstractApexRule { private static final String CONFIGURATION = "Configuration"; public ApexDangerousMethodsRule() { + super.addRuleChainVisit(ASTUserClass.class); setProperty(CODECLIMATE_CATEGORIES, new String[] { "Security" }); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);