From c3462be2c9b238df8fe5fe0a2a7dfc6ee7cac239 Mon Sep 17 00:00:00 2001 From: Benjamin Muskalla Date: Fri, 24 Sep 2021 15:43:43 +0200 Subject: [PATCH] Capture argument->return value flows --- .../model-generator/CaptureSummaryModels.ql | 45 +++++++++++++- .../model-generator/ModelGeneratorUtils.qll | 10 ++++ .../CaptureSummaryModels.expected | 9 +++ .../utils/model-generator/p/ParamFlow.java | 59 +++++++++++++++++++ 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 java/ql/test/utils/model-generator/p/ParamFlow.java diff --git a/java/ql/src/utils/model-generator/CaptureSummaryModels.ql b/java/ql/src/utils/model-generator/CaptureSummaryModels.ql index d2882ca4fd3..aa466b97e00 100644 --- a/java/ql/src/utils/model-generator/CaptureSummaryModels.ql +++ b/java/ql/src/utils/model-generator/CaptureSummaryModels.ql @@ -6,8 +6,13 @@ import java import ModelGeneratorUtils +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.internal.DataFlowImplCommon -string captureFlow(Callable api) { result = captureQualifierFlow(api) } +string captureFlow(Callable api) { + result = captureQualifierFlow(api) or + result = captureParameterFlowToReturnValue(api) +} string captureQualifierFlow(Callable api) { exists(ReturnStmt rtn | @@ -17,6 +22,44 @@ string captureQualifierFlow(Callable api) { result = asValueModel(api, "Argument[-1]", "ReturnValue") } +class ParameterToReturnValueTaintConfig extends TaintTracking::Configuration { + ParameterToReturnValueTaintConfig() { this = "ParameterToReturnValueTaintConfig" } + + override predicate isSource(DataFlow::Node source) { + exists(Parameter p, Callable api | + p = source.asParameter() and + api = p.getCallable() and + ( + not api.getReturnType() instanceof PrimitiveType and + not p.getType() instanceof PrimitiveType + ) and + ( + not api.getReturnType() instanceof TypeClass and + not p.getType() instanceof TypeClass + ) + ) + } + + override predicate isSink(DataFlow::Node sink) { sink instanceof ReturnNodeExt } +} + +// TODO: rtn -> Node as ReturnNodeExt is also PostUpdateNode, might be able to merge with p2p flow +predicate paramFlowToReturnValueExists(Parameter p) { + exists(ParameterToReturnValueTaintConfig config, ReturnStmt rtn | + rtn.getEnclosingCallable() = p.getCallable() and + config.hasFlow(DataFlow::parameterNode(p), DataFlow::exprNode(rtn.getResult())) + ) +} + +string captureParameterFlowToReturnValue(Callable api) { + exists(Parameter p | + p = api.getAParameter() and + paramFlowToReturnValueExists(p) + | + result = asTaintModel(api, parameterAccess(p), "ReturnValue") + ) +} + // TODO: handle cases like Ticker // TODO: "com.google.common.base;Converter;true;convertAll;(Iterable);;Element of Argument[0];Element of ReturnValue;taint", // TODO: infer interface from multiple implementations? e.g. UriComponentsContributor diff --git a/java/ql/src/utils/model-generator/ModelGeneratorUtils.qll b/java/ql/src/utils/model-generator/ModelGeneratorUtils.qll index 2d28c7101c6..8e0842f6fe0 100644 --- a/java/ql/src/utils/model-generator/ModelGeneratorUtils.qll +++ b/java/ql/src/utils/model-generator/ModelGeneratorUtils.qll @@ -1,5 +1,6 @@ import java import semmle.code.java.dataflow.ExternalFlow +import semmle.code.java.dataflow.internal.ContainerFlow string isExtensible(RefType ref) { if ref.isFinal() then result = "false" else result = "true" } @@ -26,3 +27,12 @@ string asSummaryModel(Callable api, string input, string output, string kind) { + output + ";" // + kind + ";" // } + +string parameterAccess(Parameter p) { + if p.getType() instanceof Array + then result = "ArrayElement of Argument[" + p.getPosition() + "]" + else + if p.getType() instanceof ContainerType + then result = "Element of Argument[" + p.getPosition() + "]" + else result = "Argument[" + p.getPosition() + "]" +} diff --git a/java/ql/test/utils/model-generator/CaptureSummaryModels.expected b/java/ql/test/utils/model-generator/CaptureSummaryModels.expected index b1f7d8875d1..fe7b795dd70 100644 --- a/java/ql/test/utils/model-generator/CaptureSummaryModels.expected +++ b/java/ql/test/utils/model-generator/CaptureSummaryModels.expected @@ -2,3 +2,12 @@ | p;FluentAPI;false;returnsThis;(String);;Argument[-1];ReturnValue;value; | | p;InnerClasses$CaptureMe;true;yesCm;(String);;Argument[0];ReturnValue;taint; | | p;InnerClasses;true;yes;(String);;Argument[0];ReturnValue;taint; | +| p;ParamFlow;true;addTo;(String,List);;Argument[0];Element of Argument[1];taint; | +| p;ParamFlow;true;returnArrayElement;(String[]);;ArrayElement of Argument[0];ReturnValue;taint; | +| p;ParamFlow;true;returnCollectionElement;(List);;Element of Argument[0];ReturnValue;taint; | +| p;ParamFlow;true;returnIterableElement;(Iterable);;Element of Argument[0];ReturnValue;taint; | +| p;ParamFlow;true;returnIteratorElement;(Iterator);;Element of Argument[0];ReturnValue;taint; | +| p;ParamFlow;true;returnMultipleParameters;(String,String);;Argument[0];ReturnValue;taint; | +| p;ParamFlow;true;returnMultipleParameters;(String,String);;Argument[1];ReturnValue;taint; | +| p;ParamFlow;true;returnVarArgElement;(String[]);;ArrayElement of Argument[0];ReturnValue;taint; | +| p;ParamFlow;true;returnsInput;(String);;Argument[0];ReturnValue;taint; | \ No newline at end of file diff --git a/java/ql/test/utils/model-generator/p/ParamFlow.java b/java/ql/test/utils/model-generator/p/ParamFlow.java new file mode 100644 index 00000000000..2b2846c1f9e --- /dev/null +++ b/java/ql/test/utils/model-generator/p/ParamFlow.java @@ -0,0 +1,59 @@ +package p; + +import java.util.Iterator; +import java.util.List; +import java.io.IOException; +import java.io.OutputStream; + + +public class ParamFlow { + + public String returnsInput(String input) { + return input; + } + + public int ignorePrimitiveReturnValue(String input) { + return input.length(); + } + + public String returnMultipleParameters(String one, String two) { + if (System.currentTimeMillis() > 100) { + return two; + } + return one; + } + + public String returnArrayElement(String[] input) { + return input[0]; + } + + public String returnVarArgElement(String... input) { + return input[0]; + } + + public String returnCollectionElement(List input) { + return input.get(0); + } + + public String returnIteratorElement(Iterator input) { + return input.next(); + } + + public String returnIterableElement(Iterable input) { + return input.iterator().next(); + } + + public Class mapType(Class input) { + return input; + } + + public void writeChunked(byte[] data, OutputStream output) + throws IOException { + output.write(data, 0, data.length); + } + + public void addTo(String data, List target) { + target.add(data); + } + +} \ No newline at end of file