From 281f25403d5d427cbaf27a296d0071def6c8c848 Mon Sep 17 00:00:00 2001 From: Benjamin Muskalla Date: Tue, 26 Oct 2021 10:41:10 +0200 Subject: [PATCH] Match enclosing unit without casting to specific nodes --- .../utils/model-generator/CaptureSinkModels.ql | 9 ++++++--- .../utils/model-generator/CaptureSourceModels.ql | 4 ++-- .../utils/model-generator/CaptureSummaryModels.ql | 15 ++++++++++----- .../model-generator/CaptureSummaryModels.expected | 1 + java/ql/test/utils/model-generator/p/Factory.java | 8 ++++++++ .../utils/model-generator/p/ImmutablePojo.java | 4 ++++ .../utils/model-generator/p/MultipleImpls.java | 4 ---- 7 files changed, 31 insertions(+), 14 deletions(-) diff --git a/java/ql/src/utils/model-generator/CaptureSinkModels.ql b/java/ql/src/utils/model-generator/CaptureSinkModels.ql index 5cf055fd591..e767f7c48f6 100644 --- a/java/ql/src/utils/model-generator/CaptureSinkModels.ql +++ b/java/ql/src/utils/model-generator/CaptureSinkModels.ql @@ -16,8 +16,8 @@ class PropagateToSinkConfiguration extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof DataFlow::ParameterNode and - source.asParameter().getCallable().isPublic() and - source.asParameter().getCallable().getDeclaringType().isPublic() and + source.getEnclosingCallable().isPublic() and + source.getEnclosingCallable().getDeclaringType().isPublic() and isRelevantForModels(source.getEnclosingCallable()) } @@ -25,7 +25,10 @@ class PropagateToSinkConfiguration extends TaintTracking::Configuration { } string asInputArgument(DataFlow::Node source) { - result = "Argument[" + source.asParameter().getPosition() + "]" + exists(int pos | + source.(DataFlow::ParameterNode).isParameterOf(_, pos) and + result = "Argument[" + pos + "]" + ) } string captureSink(Callable api) { diff --git a/java/ql/src/utils/model-generator/CaptureSourceModels.ql b/java/ql/src/utils/model-generator/CaptureSourceModels.ql index 9c14fe05c9c..93017b2eaf8 100644 --- a/java/ql/src/utils/model-generator/CaptureSourceModels.ql +++ b/java/ql/src/utils/model-generator/CaptureSourceModels.ql @@ -22,7 +22,7 @@ class FromSourceConfiguration extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node sink) { exists(Callable c | sink instanceof ReturnNodeExt and - sink.asExpr().getEnclosingCallable() = c and + sink.getEnclosingCallable() = c and c.isPublic() and c.fromSource() ) @@ -42,7 +42,7 @@ string captureSource(Callable api) { | config.hasFlow(src, sink) and specificSourceNode(sink, output, kind) and - api = src.asExpr().getEnclosingCallable() and + api = src.getEnclosingCallable() and result = asSourceModel(api, output, kind) ) } diff --git a/java/ql/src/utils/model-generator/CaptureSummaryModels.ql b/java/ql/src/utils/model-generator/CaptureSummaryModels.ql index 7ab4a5c73ff..58ebbd703f7 100644 --- a/java/ql/src/utils/model-generator/CaptureSummaryModels.ql +++ b/java/ql/src/utils/model-generator/CaptureSummaryModels.ql @@ -28,14 +28,15 @@ string captureQualifierFlow(Callable api) { } string captureFieldFlow(Callable api) { - exists(FieldAccess fa, ReturnNodeExt postUpdate | + exists(FieldAccess fa, ReturnNodeExt returnNode | not (fa.getField().isStatic() and fa.getField().isFinal()) and - postUpdate.getEnclosingCallable() = api and + returnNode.getEnclosingCallable() = api and + fa.getCompilationUnit() = api.getCompilationUnit() and isRelevantType(api.getReturnType()) and not api.getDeclaringType() instanceof EnumType and - TaintTracking::localTaint(DataFlow::exprNode(fa), postUpdate) + TaintTracking::localTaint(DataFlow::exprNode(fa), returnNode) | - result = asTaintModel(api, "Argument[-1]", asOutput(api, postUpdate)) + result = asTaintModel(api, "Argument[-1]", asOutput(api, returnNode)) ) } @@ -59,7 +60,11 @@ class ParameterToFieldConfig extends TaintTracking::Configuration { } override predicate isSink(DataFlow::Node sink) { - exists(FieldAssignment a | a.getSource() = sink.asExpr()) + exists(FieldAssignment a | + a.getSource() = sink.asExpr() and + a.getDest().(VarAccess).getVariable().getCompilationUnit() = + sink.getEnclosingCallable().getCompilationUnit() + ) } } diff --git a/java/ql/test/utils/model-generator/CaptureSummaryModels.expected b/java/ql/test/utils/model-generator/CaptureSummaryModels.expected index 187af5fa784..143c938cdec 100644 --- a/java/ql/test/utils/model-generator/CaptureSummaryModels.expected +++ b/java/ql/test/utils/model-generator/CaptureSummaryModels.expected @@ -1,5 +1,6 @@ | p;Factory;false;create;(String);;Argument[0];Argument[-1];taint; | | p;Factory;false;create;(String,int);;Argument[0];Argument[-1];taint; | +| p;Factory;false;getValue;();;Argument[-1];ReturnValue;taint; | | p;FinalClass;false;returnsInput;(String);;Argument[0];ReturnValue;taint; | | p;FluentAPI;false;returnsThis;(String);;Argument[-1];ReturnValue;value; | | p;ImmutablePojo;false;ImmutablePojo;(String,int);;Argument[0];Argument[-1];taint; | diff --git a/java/ql/test/utils/model-generator/p/Factory.java b/java/ql/test/utils/model-generator/p/Factory.java index c74b9380be4..a6e7ce5fff6 100644 --- a/java/ql/test/utils/model-generator/p/Factory.java +++ b/java/ql/test/utils/model-generator/p/Factory.java @@ -19,4 +19,12 @@ public final class Factory { this.intValue = intValue; } + public String getValue() { + return value; + } + + public int getIntValue() { + return intValue; + } + } \ No newline at end of file diff --git a/java/ql/test/utils/model-generator/p/ImmutablePojo.java b/java/ql/test/utils/model-generator/p/ImmutablePojo.java index 2dfcbf31558..660c1970bd3 100644 --- a/java/ql/test/utils/model-generator/p/ImmutablePojo.java +++ b/java/ql/test/utils/model-generator/p/ImmutablePojo.java @@ -15,6 +15,10 @@ public final class ImmutablePojo { return value; } + public long getX() { + return x; + } + public String or(String defaultValue) { return value != null ? value : defaultValue; } diff --git a/java/ql/test/utils/model-generator/p/MultipleImpls.java b/java/ql/test/utils/model-generator/p/MultipleImpls.java index 8824e7da3b2..a6697393dbc 100644 --- a/java/ql/test/utils/model-generator/p/MultipleImpls.java +++ b/java/ql/test/utils/model-generator/p/MultipleImpls.java @@ -1,9 +1,5 @@ package p; -import java.io.File; -import java.io.FileFilter; -import java.io.IOException; -import java.nio.file.Files; import java.util.concurrent.Callable; public class MultipleImpls {