From 9c85a788849bc65de245c75991b782c6d22ff712 Mon Sep 17 00:00:00 2001
From: Sergey <sergey.gorbaty@salesforce.com>
Date: Wed, 4 Jan 2017 13:33:06 -0800
Subject: [PATCH] Improving open redirect detection for strings prefixed with /

---
 .../rule/security/ApexOpenRedirectRule.java   | 24 ++++++++++++--
 .../rule/security/xml/ApexOpenRedirect.xml    | 31 +++++++++++++++++++
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java
index 2db134d25d..63ec1d3c36 100644
--- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java
+++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java
@@ -64,15 +64,33 @@ public class ApexOpenRedirectRule extends AbstractApexRule {
     }
 
     private void findSafeLiterals(AbstractApexNode<?> node) {
+        ASTBinaryExpression binaryExp = node.getFirstChildOfType(ASTBinaryExpression.class);
+        if (binaryExp != null) {
+            findSafeLiterals(binaryExp);
+        }
+
         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(node);
+                } else {
+                    ASTVariableDeclaration parent = node.getFirstParentOfType(ASTVariableDeclaration.class);
+                    addVariable(parent);
+
+                }
             }
         }
     }
 
+    private void addVariable(AbstractApexNode<?> node) {
+        ASTVariableExpression variable = node.getFirstChildOfType(ASTVariableExpression.class);
+        if (variable != null) {
+            listOfStringLiteralVariables.add(Helper.getFQVariableName(variable));
+        }
+    }
+
     /**
      * Traverses all new declarations to find PageReferences
      * 
diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexOpenRedirect.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexOpenRedirect.xml
index 74cca0be67..8278b975cd 100644
--- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexOpenRedirect.xml
+++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexOpenRedirect.xml
@@ -136,8 +136,39 @@ public class Foo {
 	static PageReference redirect() {
 	    return pr;
 	}
+}
+		]]></code>
+	</test-code> 
+	
+	<test-code>
+		<description>Unsafe pageReference object</description>
+		<expected-problems>1</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>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-data>