From 61f4253dd252591717ab79f52a5b625fd80d3289 Mon Sep 17 00:00:00 2001 From: Samuel Hopstock Date: Fri, 14 Aug 2020 16:19:02 +0200 Subject: [PATCH] #135 create dummies for explicit constructor invocations, revamp fptr dummy behavior --- .../frontends/java/JavaLanguageFrontend.java | 7 +- .../cpg/frontends/java/StatementAnalyzer.java | 12 +- .../cpg/graph/ConstructorDeclaration.java | 15 ++- .../aisec/cpg/passes/CallResolver.java | 126 ++++++------------ .../cpg/passes/VariableUsageResolver.java | 34 +++-- .../calls/FunctionPointerTest.java | 38 ++---- 6 files changed, 89 insertions(+), 143 deletions(-) diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java index 20b5d43e12..b517bde4dd 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/JavaLanguageFrontend.java @@ -60,7 +60,9 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -291,8 +293,7 @@ public String recoverTypeFromUnsolvedException(Throwable ex) { qualifier = ((UnsolvedSymbolException) ex.getCause()).getName(); } // this comes from the Javaparser! - if (qualifier.startsWith("We are unable to find the value declaration corresponding to") - || qualifier.startsWith("Solving ")) { + if (qualifier.startsWith("We are unable to find") || qualifier.startsWith("Solving ")) { return null; } String fromImport = getQualifiedNameFromImports(qualifier); diff --git a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java index a9f596a904..df229735df 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/frontends/java/StatementAnalyzer.java @@ -557,12 +557,12 @@ public SwitchStatement handleSwitchStatement(Statement stmt) { private ExplicitConstructorInvocation handleExplicitConstructorInvocation(Statement stmt) { ExplicitConstructorInvocationStmt eciStatement = stmt.asExplicitConstructorInvocationStmt(); - String containingClass; - try { - containingClass = eciStatement.resolve().declaringType().getQualifiedName(); - } catch (RuntimeException | NoClassDefFoundError e) { - containingClass = lang.recoverTypeFromUnsolvedException(e); - // base can be null here + String containingClass = ""; + RecordDeclaration currentRecord = lang.getScopeManager().getCurrentRecord(); + if (currentRecord == null) { + log.error("Explicit constructor invocation has to be located inside a record declaration!"); + } else { + containingClass = currentRecord.getName(); } ExplicitConstructorInvocation node = diff --git a/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java b/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java index 7a6efb08bd..f99e66234c 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/graph/ConstructorDeclaration.java @@ -27,6 +27,7 @@ package de.fraunhofer.aisec.cpg.graph; import de.fraunhofer.aisec.cpg.graph.type.TypeParser; +import org.checkerframework.checker.nullness.qual.Nullable; /** * The declaration of a constructor within a {@link RecordDeclaration}. Is it essentially a special @@ -47,11 +48,6 @@ public static ConstructorDeclaration from(MethodDeclaration methodDeclaration) { NodeBuilder.newConstructorDeclaration( methodDeclaration.getName(), methodDeclaration.getCode(), recordDeclaration); - if (recordDeclaration != null) { - // constructors always have implicitly the return type of their class - c.setType(TypeParser.createFrom(recordDeclaration.getName(), true)); - } - c.setBody(methodDeclaration.getBody()); c.setLocation(methodDeclaration.getLocation()); c.setParameters(methodDeclaration.getParameters()); @@ -66,4 +62,13 @@ public static ConstructorDeclaration from(MethodDeclaration methodDeclaration) { return c; } + + @Override + public void setRecordDeclaration(@Nullable RecordDeclaration recordDeclaration) { + super.setRecordDeclaration(recordDeclaration); + if (recordDeclaration != null) { + // constructors always have implicitly the return type of their class + setType(TypeParser.createFrom(recordDeclaration.getName(), true)); + } + } } diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java index ac0d7a604e..e0d22db260 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/CallResolver.java @@ -332,56 +332,40 @@ private void resolveConstructExpression(ConstructExpression constructExpression) if (record != null && record.getCode() != null && !record.getCode().isEmpty()) { ConstructorDeclaration constructor = getConstructorDeclaration(signature, record); - if (constructor != null) { - constructExpression.setConstructor(constructor); - } else { - LOGGER.warn( - "Unexpected: Could not find constructor for {} with signature {}", - record.getName(), - signature); - } + constructExpression.setConstructor(constructor); } } private void handleFunctionPointerCall(CallExpression call, Node pointer) { + if (!(pointer instanceof HasType + && ((HasType) pointer).getType() instanceof FunctionPointerType)) { + LOGGER.error("Can't handle a function pointer call without function pointer type"); + return; + } + FunctionPointerType pointerType = (FunctionPointerType) ((HasType) pointer).getType(); List invocationCandidates = new ArrayList<>(); Deque worklist = new ArrayDeque<>(); Set seen = Collections.newSetFromMap(new IdentityHashMap<>()); worklist.push(pointer); - DeclaredReferenceExpression finalReference = null; while (!worklist.isEmpty()) { Node curr = worklist.pop(); if (!seen.add(curr)) { continue; } if (curr instanceof FunctionDeclaration) { - if (((FunctionDeclaration) curr).hasSignature(call.getSignature())) { + FunctionDeclaration f = (FunctionDeclaration) curr; + // Even if it is a function declaration, the dataflow might just come from a situation + // where the target of a fptr is passed through via a return value. Keep searching if + // return type or signature don't match + if (TypeManager.getInstance().isSupertypeOf(pointerType.getReturnType(), f.getType()) + && f.hasSignature(pointerType.getParameters())) { invocationCandidates.add((FunctionDeclaration) curr); - } else if (curr.isImplicit()) { - // unknown function, so even if its signature does not fit, we need to make a new - // dummy that does match - if (((FunctionDeclaration) curr).hasSignature(call.getSignature())) { - invocationCandidates.add((FunctionDeclaration) curr); - // refine the referral pointer - if (finalReference != null && finalReference.getRefersTo().contains(curr)) { - finalReference.setRefersTo((ValueDeclaration) curr); - } - } else { - FunctionDeclaration dummy = - createDummyWithMatchingSignature((FunctionDeclaration) curr, call.getSignature()); - invocationCandidates.add(dummy); - // redirect the referral pointer - if (finalReference != null && finalReference.getRefersTo().contains(curr)) { - finalReference.setRefersTo(dummy); - } - } - } - } else { - if (curr instanceof DeclaredReferenceExpression) { - finalReference = (DeclaredReferenceExpression) curr; + // We have found a target. Don't follow this path any further, but still continue the + // other paths that might be left, as we could have several potential targets at runtime + continue; } - curr.getPrevDFG().forEach(worklist::push); } + curr.getPrevDFG().forEach(worklist::push); } call.setInvokes(invocationCandidates); } @@ -394,9 +378,7 @@ private void resolveExplicitConstructorInvocation(ExplicitConstructorInvocation if (record != null) { ConstructorDeclaration constructor = getConstructorDeclaration(signature, record); ArrayList invokes = new ArrayList<>(); - if (constructor != null) { - invokes.add(constructor); - } + invokes.add(constructor); eci.setInvokes(invokes); } } @@ -463,65 +445,25 @@ private void generateStaticImportDummies( } } - private Optional checkExistingDummies( - FunctionDeclaration template, List signature) { - if (template instanceof MethodDeclaration - && ((MethodDeclaration) template).getRecordDeclaration() != null) { - return ((MethodDeclaration) template) - .getRecordDeclaration().getMethods().stream() - .filter(m -> m.getName().equals(template.getName()) && m.hasSignature(signature)) - .map(FunctionDeclaration.class::cast) - .findFirst(); - } else { - if (currentTU == null) { - LOGGER.error( - "No current translation unit when trying to find matching dummy for {}", template); - return Optional.empty(); - } - return currentTU.getDeclarations().stream() - .filter(FunctionDeclaration.class::isInstance) - .map(FunctionDeclaration.class::cast) - .filter(f -> f.getName().equals(template.getName()) && f.hasSignature(signature)) - .findFirst(); - } - } - private FunctionDeclaration createDummyWithMatchingSignature( - FunctionDeclaration template, List signature) { - Optional existing = checkExistingDummies(template, signature); - if (existing.isPresent()) { - return existing.get(); - } + RecordDeclaration containingRecord, + String name, + String code, + boolean isStatic, + List signature) { List parameters = Util.createParameters(signature); - if (template instanceof MethodDeclaration) { - RecordDeclaration containingRecord = ((MethodDeclaration) template).getRecordDeclaration(); + if (containingRecord != null) { MethodDeclaration dummy = - NodeBuilder.newMethodDeclaration( - template.getName(), - template.getCode(), - ((MethodDeclaration) template).isStatic(), - containingRecord); + NodeBuilder.newMethodDeclaration(name, code, isStatic, containingRecord); dummy.setImplicit(true); dummy.setParameters(parameters); - if (containingRecord == null) { - // not inside a class, lets put it inside the translation unit - if (currentTU == null) { - LOGGER.error( - "No current translation unit when trying to generate method dummy {}", - dummy.getName()); - } else { - currentTU.getDeclarations().add(dummy); - } - } else { - containingRecord.getMethods().add(dummy); - } + containingRecord.getMethods().add(dummy); return dummy; } else { // function declaration, not inside a class - FunctionDeclaration dummy = - NodeBuilder.newFunctionDeclaration(template.getName(), template.getCode()); + FunctionDeclaration dummy = NodeBuilder.newFunctionDeclaration(name, code); dummy.setParameters(parameters); dummy.setImplicit(true); if (currentTU == null) { @@ -535,6 +477,16 @@ private FunctionDeclaration createDummyWithMatchingSignature( } } + private ConstructorDeclaration createConstructorDummy( + @NonNull RecordDeclaration containingRecord, List signature) { + ConstructorDeclaration dummy = + NodeBuilder.newConstructorDeclaration(containingRecord.getName(), "", containingRecord); + dummy.setImplicit(true); + dummy.setParameters(Util.createParameters(signature)); + containingRecord.getConstructors().add(dummy); + return dummy; + } + private Set getPossibleContainingTypes(Node node, RecordDeclaration curClass) { Set possibleTypes = new HashSet<>(); if (node instanceof MemberCallExpression) { @@ -595,12 +547,12 @@ private Set getOverridingCandidates( .collect(Collectors.toSet()); } - @Nullable + @NonNull private ConstructorDeclaration getConstructorDeclaration( List signature, RecordDeclaration record) { return record.getConstructors().stream() .filter(f -> f.hasSignature(signature)) .findFirst() - .orElse(null); + .orElseGet(() -> createConstructorDummy(record, signature)); } } diff --git a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java index 49e54a32e4..ca0af6ec87 100644 --- a/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java +++ b/src/main/java/de/fraunhofer/aisec/cpg/passes/VariableUsageResolver.java @@ -175,10 +175,12 @@ private Set resolveFunctionPtr( } if (containingClass == null) { - targets.add(handleUnknownMethod(functionName, reference.getType())); + targets.add( + handleUnknownMethod(functionName, fptrType.getReturnType(), fptrType.getParameters())); } else { MethodDeclaration resolved = - handleUnknownClassMethod(containingClass, functionName, reference.getType()); + handleUnknownClassMethod( + containingClass, functionName, fptrType.getReturnType(), fptrType.getParameters()); if (resolved != null) { targets.add(resolved); } @@ -385,18 +387,24 @@ private FieldDeclaration handleUnknownField(Type base, String name, Type type) { } } - private MethodDeclaration handleUnknownClassMethod(Type base, String name, Type type) { + private MethodDeclaration handleUnknownClassMethod( + Type base, String name, Type returnType, List signature) { if (!recordMap.containsKey(base)) { return null; } RecordDeclaration containingRecord = recordMap.get(base); List declarations = containingRecord.getMethods(); Optional target = - declarations.stream().filter(f -> f.getName().equals(name)).findFirst(); + declarations.stream() + .filter(f -> f.getName().equals(name)) + .filter(f -> f.getType().equals(returnType)) + .filter(f -> f.hasSignature(signature)) + .findFirst(); if (target.isEmpty()) { MethodDeclaration declaration = NodeBuilder.newMethodDeclaration(name, "", false, containingRecord); - declaration.setType(type); + declaration.setType(returnType); + declaration.setParameters(Util.createParameters(signature)); declarations.add(declaration); declaration.setImplicit(true); return declaration; @@ -405,23 +413,21 @@ private MethodDeclaration handleUnknownClassMethod(Type base, String name, Type } } - private FunctionDeclaration handleUnknownMethod(String name, Type type) { + private FunctionDeclaration handleUnknownMethod( + String name, Type returnType, List signature) { Optional target = currTu.getDeclarations().stream() .filter(FunctionDeclaration.class::isInstance) .map(FunctionDeclaration.class::cast) .filter(f -> f.getName().equals(name)) - .filter(f -> f.hasSignature(((FunctionPointerType) type).getParameters())) + .filter(f -> f.getType().equals(returnType)) + .filter(f -> f.hasSignature(signature)) .findFirst(); if (target.isEmpty()) { FunctionDeclaration declaration = NodeBuilder.newFunctionDeclaration(name, ""); - if (type instanceof FunctionPointerType) { - declaration.setType(((FunctionPointerType) type).getReturnType()); - declaration.setParameters( - Util.createParameters(((FunctionPointerType) type).getParameters())); - } else { - declaration.setType(type); - } + declaration.setType(returnType); + declaration.setParameters(Util.createParameters(signature)); + currTu.add(declaration); declaration.setImplicit(true); return declaration; diff --git a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java index b24775666a..376e347320 100644 --- a/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java +++ b/src/test/java/de/fraunhofer/aisec/cpg/enhancements/calls/FunctionPointerTest.java @@ -32,21 +32,11 @@ import de.fraunhofer.aisec.cpg.TestUtils; import de.fraunhofer.aisec.cpg.TranslationConfiguration; import de.fraunhofer.aisec.cpg.TranslationManager; -import de.fraunhofer.aisec.cpg.graph.CallExpression; -import de.fraunhofer.aisec.cpg.graph.FunctionDeclaration; -import de.fraunhofer.aisec.cpg.graph.Node; -import de.fraunhofer.aisec.cpg.graph.TranslationUnitDeclaration; -import de.fraunhofer.aisec.cpg.graph.VariableDeclaration; +import de.fraunhofer.aisec.cpg.graph.*; import java.io.File; import java.nio.file.Files; import java.nio.file.Path; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Deque; -import java.util.IdentityHashMap; -import java.util.List; -import java.util.Set; +import java.util.*; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -85,25 +75,17 @@ void test(String language) throws Exception { FunctionDeclaration main = TestUtils.findByUniqueName(functions, "main"); List calls = TestUtils.subnodesOfType(main, CallExpression.class); FunctionDeclaration noParam = - functions.stream() - .filter(f -> f.getName().equals("target") && f.getParameters().isEmpty()) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("target") && f.getParameters().isEmpty()); FunctionDeclaration singleParam = - functions.stream() - .filter(f -> f.getName().equals("target") && f.getParameters().size() == 1) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("target") && f.getParameters().size() == 1); FunctionDeclaration noParamUnknown = - functions.stream() - .filter(f -> f.getName().equals("fun") && f.getParameters().isEmpty()) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("fun") && f.getParameters().isEmpty()); FunctionDeclaration singleParamUnknown = - functions.stream() - .filter(f -> f.getName().equals("fun") && f.getParameters().size() == 1) - .findFirst() - .orElseThrow(); + TestUtils.findByUniquePredicate( + functions, f -> f.getName().equals("fun") && f.getParameters().size() == 1); Pattern pattern = Pattern.compile("\\((?.+)?\\*(?.+\\.)?(?.+)\\)"); for (CallExpression call : calls) { String func;