diff --git a/CryptoAnalysis/pom.xml b/CryptoAnalysis/pom.xml index 2564a8242..0d241802f 100644 --- a/CryptoAnalysis/pom.xml +++ b/CryptoAnalysis/pom.xml @@ -308,7 +308,6 @@ org.bouncycastle bcprov-jdk18on 1.77 - test diff --git a/CryptoAnalysis/src/main/java/crypto/HeadlessCryptoScanner.java b/CryptoAnalysis/src/main/java/crypto/HeadlessCryptoScanner.java index a98139b5c..5bbc3a8fd 100644 --- a/CryptoAnalysis/src/main/java/crypto/HeadlessCryptoScanner.java +++ b/CryptoAnalysis/src/main/java/crypto/HeadlessCryptoScanner.java @@ -17,7 +17,9 @@ import crypto.exceptions.CryptoAnalysisException; import crypto.exceptions.CryptoAnalysisParserException; import crypto.preanalysis.ExceptionAwareTransformer; +import crypto.preanalysis.IdentityTransformer; import crypto.preanalysis.SeedFactory; +import crypto.preanalysis.SeedGenerationTransformer; import crypto.providerdetection.ProviderDetection; import crypto.reporting.CSVReporter; import crypto.reporting.CSVSummaryReporter; @@ -118,6 +120,7 @@ public void exec() { if(isPreAnalysis()){ try { initializeSootWithEntryPointAllReachable(false); + setupTransformer(); } catch (CryptoAnalysisException e) { LOGGER.error("Error happened when executing HeadlessCryptoScanner.", e); } @@ -129,10 +132,10 @@ public void exec() { LOGGER.info("Using call graph algorithm {}", callGraphAlgorithm()); try { initializeSootWithEntryPointAllReachable(true); + setupTransformer(); } catch (CryptoAnalysisException e) { LOGGER.error("Error happened when executing HeadlessCryptoScanner.", e); } - ExceptionAwareTransformer.setup(rules); LOGGER.info("Analysis soot setup done in {} ", stopwatch); analyse(); LOGGER.info("Analysis finished in {}", stopwatch); @@ -142,6 +145,13 @@ public void exec() { public boolean hasSeeds(){ return hasSeeds; } + + private void setupTransformer() { + SeedGenerationTransformer.setup(rules); + IdentityTransformer.setup(); + ExceptionAwareTransformer.setup(rules); + PackManager.v().runBodyPacks(); + } private void checkIfUsesObject() { final SeedFactory seedFactory = new SeedFactory(HeadlessCryptoScanner.rules); @@ -326,6 +336,12 @@ private void initializeSootWithEntryPointAllReachable(boolean wholeProgram) thro Options.v().set_no_bodies_for_excluded(true); Options.v().set_allow_phantom_refs(true); Options.v().set_keep_line_number(true); + + Options.v().setPhaseOption("jb", "use-original-names:true"); + + // enabled to deal with method chaining + Options.v().setPhaseOption("jb.lp", "enabled:true"); + // JAVA 8 if(getJavaVersion() < 9) { @@ -370,7 +386,7 @@ private List getIncludeList() { includeList.add("java.lang.String"); includeList.add("java.lang.StringCoding"); includeList.add("java.lang.StringIndexOutOfBoundsException"); - return includeList; + return new LinkedList<>(); } private List getExcludeList() { diff --git a/CryptoAnalysis/src/main/java/crypto/analysis/AnalysisSeedWithSpecification.java b/CryptoAnalysis/src/main/java/crypto/analysis/AnalysisSeedWithSpecification.java index 3bb00f6db..a45b22c39 100644 --- a/CryptoAnalysis/src/main/java/crypto/analysis/AnalysisSeedWithSpecification.java +++ b/CryptoAnalysis/src/main/java/crypto/analysis/AnalysisSeedWithSpecification.java @@ -1,15 +1,11 @@ package crypto.analysis; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Map.Entry; -import java.util.Set; -import java.util.stream.Collectors; - +import boomerang.callgraph.ObservableICFG; +import boomerang.debugger.Debugger; +import boomerang.jimple.AllocVal; +import boomerang.jimple.Statement; +import boomerang.jimple.Val; +import boomerang.results.ForwardBoomerangResults; import com.google.common.collect.HashMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; @@ -17,20 +13,10 @@ import com.google.common.collect.Sets; import com.google.common.collect.Table; import com.google.common.collect.Table.Cell; - -import boomerang.callgraph.ObservableICFG; -import boomerang.debugger.Debugger; -import boomerang.jimple.AllocVal; -import boomerang.jimple.Statement; -import boomerang.jimple.Val; -import boomerang.results.ForwardBoomerangResults; -import crypto.analysis.errors.ForbiddenPredicateError; import crypto.analysis.errors.IncompleteOperationError; -import crypto.analysis.errors.RequiredPredicateError; import crypto.analysis.errors.TypestateError; import crypto.constraints.ConstraintSolver; import crypto.constraints.EvaluableConstraint; -import crypto.extractparameter.CallSiteWithExtractedValue; import crypto.extractparameter.CallSiteWithParamIndex; import crypto.extractparameter.ExtractParameterAnalysis; import crypto.extractparameter.ExtractedValue; @@ -67,6 +53,16 @@ import typestate.finiteautomata.ITransition; import typestate.finiteautomata.State; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.stream.Collectors; + public class AnalysisSeedWithSpecification extends IAnalysisSeed { private final ClassSpecification spec; @@ -118,9 +114,10 @@ public String toString() { public void execute() { cryptoScanner.getAnalysisListener().seedStarted(this); runTypestateAnalysis(); - if (results == null) + if (results == null) { // Timeout occured. return; + } allCallsOnObject = results.getInvokedMethodOnInstance(); runExtractParameterAnalysis(); checkInternalConstraints(); diff --git a/CryptoAnalysis/src/main/java/crypto/preanalysis/IdentityTransformer.java b/CryptoAnalysis/src/main/java/crypto/preanalysis/IdentityTransformer.java new file mode 100644 index 000000000..4411f8579 --- /dev/null +++ b/CryptoAnalysis/src/main/java/crypto/preanalysis/IdentityTransformer.java @@ -0,0 +1,69 @@ +package crypto.preanalysis; + +import soot.Body; +import soot.BodyTransformer; +import soot.PackManager; +import soot.PhaseOptions; +import soot.Transform; +import soot.UnitPatchingChain; +import soot.jimple.AssignStmt; +import soot.jimple.InstanceInvokeExpr; +import soot.jimple.internal.JInvokeStmt; +import soot.jimple.internal.JimpleLocal; + +import java.util.Map; + +/** + * This transformer removes redundant jimple body statements that are introduced by + * the jb.lp phase. These statements include identity statements as v = v and + * v = v.method(). Without this transformer, Boomerang is not able to instantiate + * correct queries. + */ +public class IdentityTransformer extends BodyTransformer { + + public static void setup() { + String phaseName = "jtp.itr"; + PackManager.v().getPack("jtp").remove(phaseName); + PackManager.v().getPack("jtp").add(new Transform(phaseName, new IdentityTransformer())); + PhaseOptions.v().setPhaseOption(phaseName, "on"); + } + + @Override + protected void internalTransform(Body body, String phaseName, Map options) { + if (body.getMethod().getDeclaringClass().getName().startsWith("java.")) { + return; + } + if (!body.getMethod().getDeclaringClass().isApplicationClass()) { + return; + } + + UnitPatchingChain units = body.getUnits(); + units.snapshotIterator().forEachRemaining(unit -> { + if (!(unit instanceof AssignStmt)) { + return; + } + + AssignStmt assignStmt = (AssignStmt) unit; + + // Identity statements for variable v: v = v + if (assignStmt.getLeftOp().equals(assignStmt.getRightOp())) { + units.remove(unit); + } + + if (!(assignStmt.getLeftOp() instanceof JimpleLocal)) { + return; + } + + if (!(assignStmt.getRightOp() instanceof InstanceInvokeExpr)) { + return; + } + + // Replace statement of the form: v = v.method() with v.method() + JimpleLocal leftSide = (JimpleLocal) assignStmt.getLeftOp(); + InstanceInvokeExpr invokeExpr = (InstanceInvokeExpr) assignStmt.getRightOp(); + if (leftSide.equals(invokeExpr.getBase())) { + units.swapWith(unit, new JInvokeStmt(invokeExpr)); + } + }); + } +} diff --git a/CryptoAnalysis/src/main/java/crypto/preanalysis/SeedGenerationTransformer.java b/CryptoAnalysis/src/main/java/crypto/preanalysis/SeedGenerationTransformer.java new file mode 100644 index 000000000..d79d99c85 --- /dev/null +++ b/CryptoAnalysis/src/main/java/crypto/preanalysis/SeedGenerationTransformer.java @@ -0,0 +1,154 @@ +package crypto.preanalysis; + +import crypto.rules.CrySLMethod; +import crypto.rules.CrySLRule; +import crypto.rules.TransitionEdge; +import soot.Body; +import soot.BodyTransformer; +import soot.PackManager; +import soot.PhaseOptions; +import soot.SootClass; +import soot.SootMethod; +import soot.Transform; +import soot.Type; +import soot.UnitPatchingChain; +import soot.jimple.InvokeExpr; +import soot.jimple.InvokeStmt; +import soot.jimple.StaticInvokeExpr; +import soot.jimple.internal.JAssignStmt; +import soot.jimple.internal.JimpleLocal; + +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; + +/** + * This transformer extends the jimple bodies, if CryptoAnalysis would not + * be able to generate seeds for some objects. Minimal programs as + * 'Cipher c = Cipher.getInstance("AES");' are transformed to jimple code + * of the form 'staticinvoke Cipher.getInstance([...])'. This transformer + * introduces a new JimpleLocal 'Cipher0' and transforms the statement to + * Cipher0 = staticinvoke Cipher.getInstance([...]). Without this transformation, + * CryptoAnalysis would not be able to generate a seed for c and would not check + * for incorrect usages. + */ +public class SeedGenerationTransformer extends BodyTransformer { + + public static void setup(Collection rules) { + final String phaseName = "jtp.sgtr"; + PackManager.v().getPack("jtp").remove(phaseName); + PackManager.v().getPack("jtp").add(new Transform(phaseName, new SeedGenerationTransformer(rules))); + PhaseOptions.v().setPhaseOption(phaseName, "on"); + } + + private final Collection rules; + private int counter; + + public SeedGenerationTransformer(Collection rules) { + this.rules = rules; + this.counter = 0; + } + + @Override + protected void internalTransform(Body body, String phaseName, Map options) { + if (body.getMethod().getDeclaringClass().getName().startsWith("java.")) { + return; + } + if (!body.getMethod().getDeclaringClass().isApplicationClass()) { + return; + } + + UnitPatchingChain units = body.getUnits(); + units.snapshotIterator().forEachRemaining(unit -> { + if (!(unit instanceof InvokeStmt)) { + return; + } + + InvokeStmt invokeStmt = (InvokeStmt) unit; + if (!(invokeStmt.getInvokeExpr() instanceof StaticInvokeExpr)) { + return; + } + + SootClass declaringClass = invokeStmt.getInvokeExpr().getMethod().getDeclaringClass(); + CrySLRule rule = getRuleForClass(declaringClass); + + if (rule == null) { + return; + } + + if (isSeedGeneratingStatement(invokeStmt.getInvokeExpr(), rule)) { + // Create new Variable + JimpleLocal leftSide = new JimpleLocal(declaringClass.getShortName() + counter, declaringClass.getType()); + body.getLocals().add(leftSide); + counter++; + + units.swapWith(unit, new JAssignStmt(leftSide, invokeStmt.getInvokeExpr())); + } + }); + } + + private CrySLRule getRuleForClass(SootClass sootClass) { + for (CrySLRule rule : rules) { + if (rule.getClassName().equals(sootClass.getName())) { + return rule; + } + } + return null; + } + + private boolean isSeedGeneratingStatement(InvokeExpr invokeExpr, CrySLRule rule) { + Collection initialTransitions = rule.getUsagePattern().getInitialTransitions(); + SootMethod method = invokeExpr.getMethod(); + + for (TransitionEdge transition : initialTransitions) { + Collection labels = transition.getLabel(); + + for (CrySLMethod label : labels) { + if (doMethodsMatch(method, rule.getClassName(), label)) { + return true; + } + } + } + + return false; + } + + private boolean doMethodsMatch(SootMethod sootMethod, String ruleClassName, CrySLMethod crySLMethod) { + String declaringSootMethodClass = sootMethod.getDeclaringClass().getName(); + if (!ruleClassName.equals(declaringSootMethodClass)) { + return false; + } + + if (!sootMethod.getName().equals(crySLMethod.getShortMethodName())) { + return false; + } + + if (!doParametersMatch(sootMethod.getParameterTypes(), crySLMethod.getParameters())) { + return false; + } + + return true; + } + + private boolean doParametersMatch(List parameterTypes, List> parameterLabels) { + if (parameterTypes.size() != parameterLabels.size()) { + return false; + } + + for (int i = 0; i < parameterTypes.size(); i++) { + if (parameterLabels.get(i).getValue().equals("AnyType")) { + continue; + } + + Type parameterType = parameterTypes.get(i); + // Soot does not track generic types, so we are required to remove <...> from the parameter + String parameterLabelType = parameterLabels.get(i).getValue().replaceAll("[<].*?[>]", ""); + + if (!parameterType.toString().equals(parameterLabelType)) { + return false; + } + } + return true; + } +} diff --git a/CryptoAnalysis/src/test/java/tests/headless/BouncyCastleHeadlessTest.java b/CryptoAnalysis/src/test/java/tests/headless/BouncyCastleHeadlessTest.java index 3bc446cb0..65ac69112 100644 --- a/CryptoAnalysis/src/test/java/tests/headless/BouncyCastleHeadlessTest.java +++ b/CryptoAnalysis/src/test/java/tests/headless/BouncyCastleHeadlessTest.java @@ -107,7 +107,7 @@ public void testBCDigestExamples() { setErrorsCount("", HardCodedError.class, 1); setErrorsCount("", RequiredPredicateError.class, 1); - setErrorsCount("", TypestateError.class, 2); + setErrorsCount("", TypestateError.class, 1); setErrorsCount("", TypestateError.class, 3); scanner.exec(); diff --git a/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoGoodusesTest.java b/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoGoodusesTest.java index 7bd64a1de..34055bfba 100644 --- a/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoGoodusesTest.java +++ b/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoGoodusesTest.java @@ -42,7 +42,7 @@ public void alwaysDefineCSPExamples() { setErrorsCount("", ConstraintError.class, 1); setErrorsCount("", RequiredPredicateError.class, 1); setErrorsCount("", IncompleteOperationError.class, 4); - setErrorsCount("", IncompleteOperationError.class, 3); + setErrorsCount("", IncompleteOperationError.class, 2); setErrorsCount("", ConstraintError.class, 1); setErrorsCount("", TypestateError.class, 1); setErrorsCount("", ConstraintError.class, 1); @@ -301,12 +301,12 @@ public void avoidInsecureHashExamples() { MavenProject mavenProject = createAndCompile(mavenProjectPath); HeadlessCryptoScanner scanner = createScanner(mavenProject); - setErrorsCount("", IncompleteOperationError.class, 1); - setErrorsCount("", IncompleteOperationError.class, 4); + setErrorsCount("", IncompleteOperationError.class, 0); + setErrorsCount("", IncompleteOperationError.class, 0); setErrorsCount("", ConstraintError.class, 1); setErrorsCount("", ConstraintError.class, 1); - setErrorsCount("", IncompleteOperationError.class, 4); - setErrorsCount("", IncompleteOperationError.class, 1); + setErrorsCount("", IncompleteOperationError.class, 0); + setErrorsCount("", IncompleteOperationError.class, 0); scanner.exec(); assertErrors(); diff --git a/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoMisusesTest.java b/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoMisusesTest.java index 409afc41c..273acb6ce 100644 --- a/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoMisusesTest.java +++ b/CryptoAnalysis/src/test/java/tests/headless/BragaCryptoMisusesTest.java @@ -33,14 +33,13 @@ public void brokenInsecureHashExamples() { MavenProject mavenProject = createAndCompile(mavenProjectPath); HeadlessCryptoScanner scanner = createScanner(mavenProject); - setErrorsCount("", IncompleteOperationError.class, 3); setErrorsCount("", ConstraintError.class, 3); - setErrorsCount("", IncompleteOperationError.class, 1); - setErrorsCount("", IncompleteOperationError.class, 1); + setErrorsCount("", IncompleteOperationError.class, 0); + setErrorsCount("", IncompleteOperationError.class, 0); setErrorsCount("", ConstraintError.class, 1); - setErrorsCount("", IncompleteOperationError.class, 1); + setErrorsCount("", IncompleteOperationError.class, 0); setErrorsCount("", ConstraintError.class, 1); - setErrorsCount("", IncompleteOperationError.class, 1); + setErrorsCount("", IncompleteOperationError.class, 0); setErrorsCount("", ConstraintError.class, 1); scanner.exec(); @@ -246,7 +245,7 @@ public void deterministicSymEncExamples() { HeadlessCryptoScanner scanner = createScanner(mavenProject); setErrorsCount("", ConstraintError.class, 6); - setErrorsCount("", IncompleteOperationError.class, 12); + setErrorsCount("", IncompleteOperationError.class, 4); setErrorsCount("", TypestateError.class, 0); setErrorsCount("", ForbiddenMethodError.class, 1); setErrorsCount("", IncompleteOperationError.class, 4); diff --git a/CryptoAnalysis/src/test/java/tests/headless/CryptoGuardTest.java b/CryptoAnalysis/src/test/java/tests/headless/CryptoGuardTest.java index 97cc8a431..f4c82dcc5 100644 --- a/CryptoAnalysis/src/test/java/tests/headless/CryptoGuardTest.java +++ b/CryptoAnalysis/src/test/java/tests/headless/CryptoGuardTest.java @@ -130,13 +130,13 @@ public void insecureAsymmetricCryptoExamples() { // This test case corresponds to the following project in CryptoGuard: // https://github.com/CryptoGuardOSS/cryptoapi-bench/blob/master/src/main/java/org/cryptoapi/bench/insecureasymmetriccrypto/InsecureAsymmetricCipherABICase1.java - setErrorsCount("", IncompleteOperationError.class, 2); + setErrorsCount("", IncompleteOperationError.class, 1); setErrorsCount("", TypestateError.class, 1); setErrorsCount("", RequiredPredicateError.class, 2); // This test case corresponds to the following project in CryptoGuard: // https://github.com/CryptoGuardOSS/cryptoapi-bench/blob/master/src/main/java/org/cryptoapi/bench/insecureasymmetriccrypto/InsecureAsymmetricCipherABICase2.java - setErrorsCount("", IncompleteOperationError.class, 2); + setErrorsCount("", IncompleteOperationError.class, 1); setErrorsCount("", RequiredPredicateError.class, 2); setErrorsCount("", ConstraintError.class, 1); // In the case above, misuse is caught correctly, but the keysize is reported to be 0 @@ -145,7 +145,7 @@ public void insecureAsymmetricCryptoExamples() { // This test case corresponds to the following project in CryptoGuard: // https://github.com/CryptoGuardOSS/cryptoapi-bench/blob/master/src/main/java/org/cryptoapi/bench/insecureasymmetriccrypto/InsecureAsymmetricCipherBB Case1.java - setErrorsCount("", IncompleteOperationError.class, 2); + setErrorsCount("", IncompleteOperationError.class, 1); setErrorsCount("", ConstraintError.class, 1); setErrorsCount("", RequiredPredicateError.class, 2); diff --git a/CryptoAnalysis/src/test/java/tests/headless/MessageDigestExampleTest.java b/CryptoAnalysis/src/test/java/tests/headless/MessageDigestExampleTest.java index 49fdf87f5..7a9d443b3 100644 --- a/CryptoAnalysis/src/test/java/tests/headless/MessageDigestExampleTest.java +++ b/CryptoAnalysis/src/test/java/tests/headless/MessageDigestExampleTest.java @@ -14,9 +14,8 @@ public void loadMessageDigestExample() { String mavenProjectPath = new File("../CryptoAnalysisTargets/MessageDigestExample").getAbsolutePath(); MavenProject mavenProject = createAndCompile(mavenProjectPath); HeadlessCryptoScanner scanner = createScanner(mavenProject); - - //false positive - setErrorsCount("", IncompleteOperationError.class, 2); + + setErrorsCount("", IncompleteOperationError.class, 1); scanner.exec(); assertErrors(); diff --git a/CryptoAnalysis/src/test/java/tests/headless/ReportedIssueTest.java b/CryptoAnalysis/src/test/java/tests/headless/ReportedIssueTest.java index ee960a023..0f412e67e 100644 --- a/CryptoAnalysis/src/test/java/tests/headless/ReportedIssueTest.java +++ b/CryptoAnalysis/src/test/java/tests/headless/ReportedIssueTest.java @@ -57,8 +57,8 @@ public void reportedIssues() { setErrorsCount("", RequiredPredicateError.class, 4); + setErrorsCount("", IncompleteOperationError.class, 1); setErrorsCount("", ConstraintError.class, 2); - setErrorsCount("", IncompleteOperationError.class, 0); scanner.exec(); assertErrors(); }