Fixing a bug in ApexBadCrypto related to inline detection

sfdcsteve · Mar 20, 2023 · d181b56 · d181b56
1 parent 013ada5
commit d181b56
Showing 2 changed files with 44 additions and 7 deletions.
diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexBadCryptoRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexBadCryptoRule.java
@@ -8,14 +8,10 @@
 import java.util.List;
 import java.util.Set;
 
-import net.sourceforge.pmd.lang.apex.ast.ASTFieldDeclaration;
-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.ApexNode;
+import net.sourceforge.pmd.lang.apex.ast.*;
 import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
 import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
+import scala.concurrent.impl.FutureConvertersImpl;
 
 /**
  * Finds encryption schemes using hardcoded IV, hardcoded key
@@ -104,7 +100,19 @@ private void validateStaticIVorKey(ASTMethodCallExpression methodCall, Object da
     }
 
     private void reportIfHardCoded(Object data, Object potentialIV) {
-        if (potentialIV instanceof ASTVariableExpression) {
+        if (potentialIV instanceof ASTMethodCallExpression) {
+            ASTMethodCallExpression expression = (ASTMethodCallExpression) potentialIV;
+            if (expression.getNumChildren()>1) {
+                Object potentialStaticIV = expression.getChild(1);
+                if (potentialStaticIV instanceof ASTLiteralExpression) {
+                    ASTLiteralExpression variable = (ASTLiteralExpression) potentialStaticIV;
+                    if (variable.isString()) {
+                        addViolation(data, variable);
+                    }
+                }
+            }
+        }
+        else if (potentialIV instanceof ASTVariableExpression) {
             ASTVariableExpression variable = (ASTVariableExpression) potentialIV;
             if (potentiallyStaticBlob.contains(Helper.getFQVariableName(variable))) {
                 addViolation(data, variable);

diff --git a/...apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexBadCrypto.xml b/...apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexBadCrypto.xml
@@ -95,6 +95,35 @@ public class Foo {
             Blob encrypted = Crypto.encrypt('AES128', key, exampleIv, data);
         }
     }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Apex Crypto inline hardcoded IV</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>5</expected-linenumbers>
+        <code><![CDATA[
+public class Foo {
+    public Foo() {
+        Blob key = Crypto.generateAesKey(128);
+        Blob data = Blob.valueOf('Data to be encrypted');
+        Blob encrypted = Crypto.encrypt('AES128', key, Blob.valueOf('0000000000000000'), data);
+    }
+}
+        ]]></code>
+    </test-code>
+    <test-code>
+        <description>Apex Crypto Inline hardcoded Key</description>
+        <expected-problems>1</expected-problems>
+        <expected-linenumbers>5</expected-linenumbers>
+        <code><![CDATA[
+public class Foo {
+    public Foo() {
+        Blob data = Blob.valueOf('Data to be encrypted');
+        Blob IV = Crypto.generateAesKey(128);
+        Blob encrypted = Crypto.encrypt('AES128', Blob.valueOf('Hard Coded Key'), IV, data);
+    }
 }
         ]]></code>
     </test-code>