Skip to content

Commit b0a24a7

Browse files
committed
C#: Change the implementation on getAnInput and getAnOutput based on hvitveds recommendations.
1 parent c2196a0 commit b0a24a7

File tree

2 files changed

+22
-15
lines changed

2 files changed

+22
-15
lines changed

csharp/ql/src/Telemetry/ExternalApi.qll

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
/** Provides classes and predicates related to handling APIs from external libraries. */
22

33
private import csharp
4+
private import semmle.code.csharp.dispatch.Dispatch
45
private import semmle.code.csharp.dataflow.DataFlow
56
private import semmle.code.csharp.dataflow.ExternalFlow
67
private import semmle.code.csharp.dataflow.FlowSummary
8+
private import semmle.code.csharp.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon
79
private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
810
private import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlowDispatch
911
private import semmle.code.csharp.dataflow.TaintTracking
@@ -57,20 +59,25 @@ class ExternalApi extends DataFlowDispatch::DataFlowCallable {
5759
string getInfo() { result = this.getInfoPrefix() + "#" + this.getSignature() }
5860

5961
/** Gets a node that is an input to a call to this API. */
60-
private DataFlow::Node getAnInput() {
61-
exists(Call call | call.getTarget().getUnboundDeclaration() = this |
62-
result.asExpr() = call.getAnArgument()
62+
private ArgumentNode getAnInput() {
63+
exists(DispatchCall call |
64+
result.getCall().(DataFlowDispatch::NonDelegateDataFlowCall).getDispatchCall() = call
65+
|
66+
this = call.getADynamicTarget().getUnboundDeclaration()
67+
or
68+
this = call.getAStaticTarget().getUnboundDeclaration()
6369
)
64-
or
65-
result.(ArgumentNode).getCall().getEnclosingCallable() = this
6670
}
6771

6872
/** Gets a node that is an output from a call to this API. */
6973
private DataFlow::Node getAnOutput() {
70-
exists(Call call | call.getTarget().getUnboundDeclaration() = this | result.asExpr() = call)
71-
or
72-
result.(PostUpdateNode).getPreUpdateNode().(ArgumentNode).getCall().getEnclosingCallable() =
73-
this
74+
exists(DataFlowDispatch::NonDelegateDataFlowCall call, DataFlowImplCommon::ReturnKindExt ret |
75+
result = ret.getAnOutNode(call)
76+
|
77+
this = call.getDispatchCall().getADynamicTarget().getUnboundDeclaration()
78+
or
79+
this = call.getDispatchCall().getAStaticTarget().getUnboundDeclaration()
80+
)
7481
}
7582

7683
/** Holds if this API has a supported summary. */
@@ -79,15 +86,15 @@ class ExternalApi extends DataFlowDispatch::DataFlowCallable {
7986
defaultAdditionalTaintStep(this.getAnInput(), _)
8087
}
8188

82-
/** Holds if this API is is a constructor without parameters */
89+
/** Holds if this API is is a constructor without parameters. */
8390
private predicate isParameterlessConstructor() {
8491
this instanceof Constructor and this.getNumberOfParameters() = 0
8592
}
8693

87-
/** Holds if this API is part of a common testing library or framework */
94+
/** Holds if this API is part of a common testing library or framework. */
8895
private predicate isTestLibrary() { this.getDeclaringType() instanceof TestLibrary }
8996

90-
/** Holds if this API is not worth supporting */
97+
/** Holds if this API is not worth supporting. */
9198
predicate isUninteresting() { this.isTestLibrary() or this.isParameterlessConstructor() }
9299

93100
/** Holds if this API is a known source. */

java/ql/src/Telemetry/ExternalApi.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,15 @@ class ExternalApi extends Callable {
7575
TaintTracking::localAdditionalTaintStep(this.getAnInput(), _)
7676
}
7777

78-
/** Holds if this API is is a constructor without parameters */
78+
/** Holds if this API is is a constructor without parameters. */
7979
private predicate isParameterlessConstructor() {
8080
this instanceof Constructor and this.getNumberOfParameters() = 0
8181
}
8282

83-
/** Holds if this API is part of a common testing library or framework */
83+
/** Holds if this API is part of a common testing library or framework. */
8484
private predicate isTestLibrary() { this.getDeclaringType() instanceof TestLibrary }
8585

86-
/** Holds if this API is not worth supporting */
86+
/** Holds if this API is not worth supporting. */
8787
predicate isUninteresting() { this.isTestLibrary() or this.isParameterlessConstructor() }
8888

8989
/** Holds if this API is a known source. */

0 commit comments

Comments
 (0)