Merge branch 'pr-173'

This commit is contained in:
Juan Martín Sotuyo Dodero
2017-01-02 15:41:11 -03:00
6 changed files with 222 additions and 0 deletions

View File

@ -0,0 +1,131 @@
/**
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.apex.rule.security;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import net.sourceforge.pmd.lang.apex.ast.ASTBinaryExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTField;
import net.sourceforge.pmd.lang.apex.ast.ASTLiteralExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
/**
* Flags usage of http request.setHeader('Authorization',..) and suggests using
* named credentials which helps store credentials for the callout in a safe
* place.
*
* @author sergey.gorbaty
*
*/
public class ApexSuggestUsingNamedCredRule extends AbstractApexRule {
private static final String SET_HEADER = "setHeader";
private static final String AUTHORIZATION = "Authorization";
private final Set<String> listOfAuthorizationVariables = new HashSet<>();
public ApexSuggestUsingNamedCredRule() {
super.addRuleChainVisit(ASTUserClass.class);
setProperty(CODECLIMATE_CATEGORIES, new String[] { "Security" });
setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100);
setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false);
}
@Override
public Object visit(ASTUserClass node, Object data) {
if (Helper.isTestMethodOrClass(node)) {
return data;
}
List<ASTVariableDeclaration> variableDecls = node.findDescendantsOfType(ASTVariableDeclaration.class);
for (ASTVariableDeclaration varDecl : variableDecls) {
findAuthLiterals(varDecl);
}
List<ASTField> fieldDecl = node.findDescendantsOfType(ASTField.class);
for (ASTField fDecl : fieldDecl) {
findFieldLiterals(fDecl);
}
List<ASTMethodCallExpression> methodCalls = node.findDescendantsOfType(ASTMethodCallExpression.class);
for (ASTMethodCallExpression method : methodCalls) {
flagAuthorizationHeaders(method, data);
}
listOfAuthorizationVariables.clear();
return data;
}
private void findFieldLiterals(final ASTField fDecl) {
Object f = fDecl.getNode().getFieldInfo().getValue();
if (f != null && f instanceof String) {
final String fieldValue = (String) f;
if (AUTHORIZATION.equalsIgnoreCase(fieldValue)) {
listOfAuthorizationVariables.add(Helper.getFQVariableName(fDecl));
}
}
}
private void flagAuthorizationHeaders(final ASTMethodCallExpression node, Object data) {
if (!Helper.isMethodName(node, SET_HEADER)) {
return;
}
final ASTBinaryExpression binaryNode = node.getFirstChildOfType(ASTBinaryExpression.class);
if (binaryNode != null) {
runChecks(binaryNode, data);
}
runChecks(node, data);
}
private void findAuthLiterals(final AbstractApexNode<?> node) {
ASTLiteralExpression literal = node.getFirstChildOfType(ASTLiteralExpression.class);
if (literal != null) {
ASTVariableExpression variable = node.getFirstChildOfType(ASTVariableExpression.class);
if (variable != null) {
if (isAuthorizationLiteral(literal)) {
listOfAuthorizationVariables.add(Helper.getFQVariableName(variable));
}
}
}
}
private void runChecks(final AbstractApexNode<?> node, Object data) {
ASTLiteralExpression literalNode = node.getFirstChildOfType(ASTLiteralExpression.class);
if (literalNode != null) {
if (isAuthorizationLiteral(literalNode)) {
addViolation(data, literalNode);
}
}
final ASTVariableExpression varNode = node.getFirstChildOfType(ASTVariableExpression.class);
if (varNode != null) {
if (listOfAuthorizationVariables.contains(Helper.getFQVariableName(varNode))) {
addViolation(data, varNode);
}
}
}
private boolean isAuthorizationLiteral(final ASTLiteralExpression literal) {
Object o = literal.getNode().getLiteral();
if (o instanceof String) {
String lit = (String) o;
if (lit.equalsIgnoreCase(AUTHORIZATION)) {
return true;
}
}
return false;
}
}

View File

@ -271,4 +271,15 @@
<property name="cc_block_highlighting" value="false"/>
</properties>
</rule>
<rule ref="rulesets/apex/security.xml/ApexSuggestUsingNamedCred" message="Consider using named credentials for authenticated callouts">
<priority>3</priority>
<properties>
<!-- relevant for Code Climate output only -->
<property name="cc_categories" value="Security"/>
<property name="cc_remediation_points_multiplier" value="20"/>
<property name="cc_block_highlighting" value="false"/>
</properties>
</rule>
</ruleset>

View File

@ -207,5 +207,27 @@ public class Foo {
]]>
</example>
</rule>
<rule name="ApexSuggestUsingNamedCred" since="5.5.3"
message="Suggest named credentials for authentication"
class="net.sourceforge.pmd.lang.apex.rule.security.ApexSuggestUsingNamedCredRule"
externalInfoUrl="${pmd.website.baseurl}/rules/apex/security.html#ApexSuggestUsingNamedCredRule">
<description>
Consider using named credentials for authenticated callouts
</description>
<priority>3</priority>
<example>
<![CDATA[
public class Foo {
public void foo(String username, String password) {
Blob headerValue = Blob.valueOf(username + ':' + password);
String authorizationHeader = 'BASIC ' + EncodingUtil.base64Encode(headerValue);
req.setHeader('Authorization', authorizationHeader);
}
}
]]>
</example>
</rule>
</ruleset>

View File

@ -22,5 +22,6 @@ public class SecurityRulesTest extends SimpleAggregatorTst {
addRule(RULESET, "ApexInsecureEndpoint");
addRule(RULESET, "ApexCRUDViolation");
addRule(RULESET, "ApexDangerousMethods");
addRule(RULESET, "ApexSuggestUsingNamedCred");
}
}

View File

@ -0,0 +1,56 @@
<?xml version="1.0" encoding="UTF-8"?>
<test-data>
<test-code>
<description>Using basic auth with literal
</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public void foo(String username, String password) {
HttpRequest req = new HttpRequest();
Blob headerValue = Blob.valueOf(username + ':' + password);
String authorizationHeader = 'BASIC ' + EncodingUtil.base64Encode(headerValue);
req.setHeader('Authorization', authorizationHeader);
}
}
]]></code>
</test-code>
<test-code>
<description>Using basic auth with variable
</description>
<expected-problems>1</expected-problems>
<code><![CDATA[
public class Foo {
public static String AUTH = 'Authorization';
public void foo(String username, String password) {
HttpRequest req = new HttpRequest();
Blob headerValue = Blob.valueOf(username + ':' + password);
String authorizationHeader = 'BASIC ' + EncodingUtil.base64Encode(headerValue);
req.setHeader(AUTH, authorizationHeader);
}
}
]]></code>
</test-code>
<test-code>
<description>Using named credentials
</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
public class Foo {
public void foo() {
HttpRequest req = new HttpRequest();
req.setEndpoint('callout:***My_Named_Credential***/some_path');
req.setMethod('GET');
Http http = new Http();
HTTPResponse res = http.send(req);
}
}
]]></code>
</test-code>
</test-data>

View File

@ -18,6 +18,7 @@ This ruleset contains links to rules that are new in PMD v5.5.3
<rule ref="rulesets/apex/security.xml/ApexSOQLInjection"/>
<rule ref="rulesets/apex/security.xml/ApexCRUDViolation"/>
<rule ref="rulesets/apex/security.xml/ApexDangerousMethods"/>
<rule ref="rulesets/apex/security.xml/ApexSuggestUsingNamedCred"/>
</ruleset>