forked from phoedos/pmd
Improving open redirect detection for static fields
This commit is contained in:

committed by
Juan Martín Sotuyo Dodero

parent
608c9139be
commit
a38526544a
@@ -3,12 +3,11 @@
|
||||
*/
|
||||
package net.sourceforge.pmd.lang.apex.rule.security;
|
||||
|
||||
import java.lang.reflect.Field;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
import apex.jorje.data.ast.Identifier;
|
||||
import apex.jorje.data.ast.TypeRef.ClassTypeRef;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTBinaryExpression;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTField;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ASTLiteralExpression;
|
||||
@@ -21,6 +20,10 @@ import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
|
||||
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
|
||||
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
|
||||
|
||||
import apex.jorje.data.ast.Identifier;
|
||||
import apex.jorje.data.ast.TypeRef.ClassTypeRef;
|
||||
import apex.jorje.semantic.symbol.member.variable.StandardFieldInfo;
|
||||
|
||||
/**
|
||||
* Looking for potential Open redirect via PageReference variable input
|
||||
*
|
||||
@@ -66,10 +69,48 @@ public class ApexOpenRedirectRule extends AbstractApexRule {
|
||||
private void findSafeLiterals(AbstractApexNode<?> node) {
|
||||
ASTLiteralExpression literal = node.getFirstChildOfType(ASTLiteralExpression.class);
|
||||
if (literal != null) {
|
||||
ASTVariableExpression variable = node.getFirstChildOfType(ASTVariableExpression.class);
|
||||
if (variable != null) {
|
||||
listOfStringLiteralVariables.add(Helper.getFQVariableName(variable));
|
||||
int index = literal.jjtGetChildIndex();
|
||||
if (index == 0) {
|
||||
if (node instanceof ASTVariableDeclaration) {
|
||||
addVariable((ASTVariableDeclaration) node);
|
||||
} else {
|
||||
ASTVariableDeclaration parent = node.getFirstParentOfType(ASTVariableDeclaration.class);
|
||||
addVariable(parent);
|
||||
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (node instanceof ASTField) {
|
||||
try {
|
||||
final Field f = node.getNode().getClass().getDeclaredField("fieldInfo");
|
||||
f.setAccessible(true);
|
||||
final StandardFieldInfo fieldInfo = (StandardFieldInfo) f.get(node.getNode());
|
||||
if (fieldInfo.getType().getApexName().equalsIgnoreCase("String")) {
|
||||
if (fieldInfo.getValue() != null) {
|
||||
addVariable(fieldInfo);
|
||||
}
|
||||
}
|
||||
|
||||
} catch (NoSuchFieldException | SecurityException | IllegalArgumentException
|
||||
| IllegalAccessException e) {
|
||||
// preventing exceptions from this code
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
private void addVariable(StandardFieldInfo fieldInfo) {
|
||||
StringBuilder sb = new StringBuilder().append(fieldInfo.getDefiningType().getApexName()).append(":")
|
||||
.append(fieldInfo.getName());
|
||||
listOfStringLiteralVariables.add(sb.toString());
|
||||
|
||||
}
|
||||
|
||||
private void addVariable(ASTVariableDeclaration node) {
|
||||
ASTVariableExpression variable = node.getFirstChildOfType(ASTVariableExpression.class);
|
||||
if (variable != null) {
|
||||
listOfStringLiteralVariables.add(Helper.getFQVariableName(variable));
|
||||
}
|
||||
}
|
||||
|
||||
|
@@ -2,7 +2,7 @@
|
||||
|
||||
<test-data>
|
||||
|
||||
<test-code>
|
||||
<test-code>
|
||||
<description>PageReference open redirect with unsafe variable
|
||||
concatenation
|
||||
</description>
|
||||
@@ -111,11 +111,11 @@ public class Foo {
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>PageReference as a field</description>
|
||||
<expected-problems>1</expected-problems>
|
||||
<description>PageReference to a literal field</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
String test1 = '';
|
||||
String test1 = 'string';
|
||||
PageReference pr = new PageReference(test1);
|
||||
|
||||
static PageReference redirect() {
|
||||
@@ -140,4 +140,35 @@ public class Foo {
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Safe pageReference object</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
|
||||
static PageReference redirect(String otherStuff) {
|
||||
String test1 = '/' + otherStuff;
|
||||
PageReference pr = new PageReference(test1);
|
||||
return pr;
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
<test-code>
|
||||
<description>Static link pageReference object</description>
|
||||
<expected-problems>0</expected-problems>
|
||||
<code><![CDATA[
|
||||
public class Foo {
|
||||
private static final String EMAILAUTHOR_URL = '/_ui/core/email/author/EmailAuthor';
|
||||
|
||||
static PageReference redirect() {
|
||||
return new PageReference(EMAILAUTHOR_URL);
|
||||
}
|
||||
}
|
||||
]]></code>
|
||||
</test-code>
|
||||
|
||||
|
||||
|
||||
</test-data>
|
||||
|
Reference in New Issue
Block a user