Skip to content

Commit 8257475

Browse files
aschackmullaschackmull
authored
Merge pull request #21132 from aschackmull/csharp/mad-barriers
C#: Add support for MaD barriers and barrier guards.
2 parents a96cd39 + 1151fc3 commit 8257475

File tree

4 files changed

+170
-7
lines changed

4 files changed

+170
-7
lines changed

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowPublic.qll‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,42 @@ module BarrierGuard<guardChecksSig/3 guardChecks> {
197197
}
198198
}
199199

200+
bindingset[this]
201+
private signature class ParamSig;
202+
203+
private module WithParam<ParamSig P> {
204+
/**
205+
* Holds if the guard `g` validates the expression `e` upon evaluating to `gv`.
206+
*
207+
* The expression `e` is expected to be a syntactic part of the guard `g`.
208+
* For example, the guard `g` might be a call `isSafe(x)` and the expression `e`
209+
* the argument `x`.
210+
*/
211+
signature predicate guardChecksSig(Guard g, Expr e, GuardValue gv, P param);
212+
}
213+
214+
/**
215+
* Provides a set of barrier nodes for a guard that validates an expression.
216+
*
217+
* This is expected to be used in `isBarrier`/`isSanitizer` definitions
218+
* in data flow and taint tracking.
219+
*/
220+
module ParameterizedBarrierGuard<ParamSig P, WithParam<P>::guardChecksSig/4 guardChecks> {
221+
private import SsaImpl as SsaImpl
222+
223+
/** Gets a node that is safely guarded by the given guard check. */
224+
pragma[nomagic]
225+
Node getABarrierNode(P param) {
226+
SsaFlow::asNode(result) =
227+
SsaImpl::DataFlowIntegration::ParameterizedBarrierGuard<P, guardChecks/4>::getABarrierNode(param)
228+
or
229+
exists(Guard g, Expr e, GuardValue v |
230+
guardChecks(g, e, v, param) and
231+
g.controlsNode(result.getControlFlowNode(), e, v)
232+
)
233+
}
234+
}
235+
200236
/**
201237
* A reference contained in an object. This is either a field, a property,
202238
* or an element in a collection.

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/ExternalFlow.qll‎

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ private import FlowSummaryImpl::Public
9797
private import FlowSummaryImpl::Private
9898
private import FlowSummaryImpl::Private::External
9999
private import semmle.code.csharp.commons.QualifiedName
100+
private import semmle.code.csharp.controlflow.Guards
100101
private import semmle.code.csharp.dispatch.OverridableCallable
101102
private import semmle.code.csharp.frameworks.System
102103
private import codeql.dataflow.internal.AccessPathSyntax as AccessPathSyntax
@@ -115,7 +116,9 @@ module ModelValidation {
115116
summaryModel(_, _, _, _, _, _, path, _, _, _, _) or
116117
summaryModel(_, _, _, _, _, _, _, path, _, _, _) or
117118
sinkModel(_, _, _, _, _, _, path, _, _, _) or
118-
sourceModel(_, _, _, _, _, _, path, _, _, _)
119+
sourceModel(_, _, _, _, _, _, path, _, _, _) or
120+
barrierModel(_, _, _, _, _, _, path, _, _, _) or
121+
barrierGuardModel(_, _, _, _, _, _, path, _, _, _, _)
119122
}
120123

121124
private module MkAccessPath = AccessPathSyntax::AccessPath<getRelevantAccessPath/1>;
@@ -128,6 +131,8 @@ module ModelValidation {
128131
exists(string pred, AccessPath input, AccessPathToken part |
129132
sinkModel(_, _, _, _, _, _, input, _, _, _) and pred = "sink"
130133
or
134+
barrierGuardModel(_, _, _, _, _, _, input, _, _, _, _) and pred = "barrier guard"
135+
or
131136
summaryModel(_, _, _, _, _, _, input, _, _, _, _) and pred = "summary"
132137
|
133138
(
@@ -150,6 +155,8 @@ module ModelValidation {
150155
exists(string pred, AccessPath output, AccessPathToken part |
151156
sourceModel(_, _, _, _, _, _, output, _, _, _) and pred = "source"
152157
or
158+
barrierModel(_, _, _, _, _, _, output, _, _, _) and pred = "barrier"
159+
or
153160
summaryModel(_, _, _, _, _, _, _, output, _, _, _) and pred = "summary"
154161
|
155162
(
@@ -167,7 +174,13 @@ module ModelValidation {
167174
private module KindValConfig implements SharedModelVal::KindValidationConfigSig {
168175
predicate summaryKind(string kind) { summaryModel(_, _, _, _, _, _, _, _, kind, _, _) }
169176

170-
predicate sinkKind(string kind) { sinkModel(_, _, _, _, _, _, _, kind, _, _) }
177+
predicate sinkKind(string kind) {
178+
sinkModel(_, _, _, _, _, _, _, kind, _, _)
179+
or
180+
barrierModel(_, _, _, _, _, _, _, kind, _, _)
181+
or
182+
barrierGuardModel(_, _, _, _, _, _, _, _, kind, _, _)
183+
}
171184

172185
predicate sourceKind(string kind) { sourceModel(_, _, _, _, _, _, _, kind, _, _) }
173186

@@ -185,6 +198,12 @@ module ModelValidation {
185198
or
186199
sinkModel(namespace, type, _, name, signature, ext, _, _, provenance, _) and pred = "sink"
187200
or
201+
barrierModel(namespace, type, _, name, signature, ext, _, _, provenance, _) and
202+
pred = "barrier"
203+
or
204+
barrierGuardModel(namespace, type, _, name, signature, ext, _, _, _, provenance, _) and
205+
pred = "barrier guard"
206+
or
188207
summaryModel(namespace, type, _, name, signature, ext, _, _, _, provenance, _) and
189208
pred = "summary"
190209
or
@@ -210,6 +229,14 @@ module ModelValidation {
210229
invalidProvenance(provenance) and
211230
result = "Unrecognized provenance description \"" + provenance + "\" in " + pred + " model."
212231
)
232+
or
233+
exists(string acceptingvalue |
234+
barrierGuardModel(_, _, _, _, _, _, _, acceptingvalue, _, _, _) and
235+
invalidAcceptingValue(acceptingvalue) and
236+
result =
237+
"Unrecognized accepting value description \"" + acceptingvalue +
238+
"\" in barrier guard model."
239+
)
213240
}
214241

215242
/** Holds if some row in a MaD flow model appears to contain typos. */
@@ -229,6 +256,10 @@ private predicate elementSpec(
229256
or
230257
sinkModel(namespace, type, subtypes, name, signature, ext, _, _, _, _)
231258
or
259+
barrierModel(namespace, type, subtypes, name, signature, ext, _, _, _, _)
260+
or
261+
barrierGuardModel(namespace, type, subtypes, name, signature, ext, _, _, _, _, _)
262+
or
232263
summaryModel(namespace, type, subtypes, name, signature, ext, _, _, _, _, _)
233264
or
234265
Extensions::neutralModel(namespace, type, name, signature, _, _) and ext = "" and subtypes = true
@@ -372,7 +403,9 @@ Declaration interpretElement(
372403
private predicate relevantExt(string ext) {
373404
summaryModel(_, _, _, _, _, ext, _, _, _, _, _) or
374405
sourceModel(_, _, _, _, _, ext, _, _, _, _) or
375-
sinkModel(_, _, _, _, _, ext, _, _, _, _)
406+
sinkModel(_, _, _, _, _, ext, _, _, _, _) or
407+
barrierModel(_, _, _, _, _, ext, _, _, _, _) or
408+
barrierGuardModel(_, _, _, _, _, ext, _, _, _, _, _)
376409
}
377410

378411
private class ExtPath = AccessPathSyntax::AccessPath<relevantExt/1>::AccessPath;
@@ -411,6 +444,53 @@ private module Cached {
411444
isSinkNode(n, kind, model) and n.asNode() = node
412445
)
413446
}
447+
448+
private newtype TKindModelPair =
449+
TMkPair(string kind, string model) { isBarrierGuardNode(_, _, kind, model) }
450+
451+
private GuardValue convertAcceptingValue(AcceptingValue av) {
452+
av.isTrue() and result.asBooleanValue() = true
453+
or
454+
av.isFalse() and result.asBooleanValue() = false
455+
or
456+
av.isNoException() and result.getDualValue().isThrowsException()
457+
or
458+
av.isZero() and result.asIntValue() = 0
459+
or
460+
av.isNotZero() and result.getDualValue().asIntValue() = 0
461+
or
462+
av.isNull() and result.isNullValue()
463+
or
464+
av.isNotNull() and result.isNonNullValue()
465+
}
466+
467+
private predicate barrierGuardChecks(Guard g, Expr e, GuardValue gv, TKindModelPair kmp) {
468+
exists(
469+
SourceSinkInterpretationInput::InterpretNode n, AcceptingValue acceptingvalue, string kind,
470+
string model
471+
|
472+
isBarrierGuardNode(n, acceptingvalue, kind, model) and
473+
n.asNode().asExpr() = e and
474+
kmp = TMkPair(kind, model) and
475+
gv = convertAcceptingValue(acceptingvalue)
476+
|
477+
g.(Call).getAnArgument() = e or g.(QualifiableExpr).getQualifier() = e
478+
)
479+
}
480+
481+
/**
482+
* Holds if `node` is specified as a barrier with the given kind in a MaD flow
483+
* model.
484+
*/
485+
cached
486+
predicate barrierNode(Node node, string kind, string model) {
487+
exists(SourceSinkInterpretationInput::InterpretNode n |
488+
isBarrierNode(n, kind, model) and n.asNode() = node
489+
)
490+
or
491+
ParameterizedBarrierGuard<TKindModelPair, barrierGuardChecks/4>::getABarrierNode(TMkPair(kind,
492+
model)) = node
493+
}
414494
}
415495

416496
import Cached
@@ -427,6 +507,12 @@ predicate sourceNode(Node node, string kind) { sourceNode(node, kind, _) }
427507
*/
428508
predicate sinkNode(Node node, string kind) { sinkNode(node, kind, _) }
429509

510+
/**
511+
* Holds if `node` is specified as a barrier with the given kind in a MaD flow
512+
* model.
513+
*/
514+
predicate barrierNode(Node node, string kind) { barrierNode(node, kind, _) }
515+
430516
private predicate isOverridableCallable(OverridableCallable c) {
431517
not exists(Type t, Callable base | c.getOverridee+() = base and t = base.getDeclaringType() |
432518
t instanceof SystemObjectClass or

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/FlowSummaryImpl.qll‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,16 +232,27 @@ module SourceSinkInterpretationInput implements
232232
}
233233

234234
predicate barrierElement(
235-
Element n, string output, string kind, Public::Provenance provenance, string model
235+
Element e, string output, string kind, Public::Provenance provenance, string model
236236
) {
237-
none()
237+
exists(
238+
string namespace, string type, boolean subtypes, string name, string signature, string ext
239+
|
240+
barrierModel(namespace, type, subtypes, name, signature, ext, output, kind, provenance, model) and
241+
e = interpretElement(namespace, type, subtypes, name, signature, ext)
242+
)
238243
}
239244

240245
predicate barrierGuardElement(
241-
Element n, string input, Public::AcceptingValue acceptingvalue, string kind,
246+
Element e, string input, Public::AcceptingValue acceptingvalue, string kind,
242247
Public::Provenance provenance, string model
243248
) {
244-
none()
249+
exists(
250+
string namespace, string type, boolean subtypes, string name, string signature, string ext
251+
|
252+
barrierGuardModel(namespace, type, subtypes, name, signature, ext, input, acceptingvalue,
253+
kind, provenance, model) and
254+
e = interpretElement(namespace, type, subtypes, name, signature, ext)
255+
)
245256
}
246257

247258
class SourceOrSinkElement = Element;

‎csharp/ql/lib/semmle/code/csharp/dataflow/internal/SsaImpl.qll‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,36 @@ private module Cached {
987987

988988
predicate getABarrierNode = getABarrierNodeImpl/0;
989989
}
990+
991+
bindingset[this]
992+
private signature class ParamSig;
993+
994+
private module WithParam<ParamSig P> {
995+
signature predicate guardChecksSig(Guards::Guard g, Expr e, Guards::GuardValue gv, P param);
996+
}
997+
998+
cached // nothing is actually cached
999+
module ParameterizedBarrierGuard<ParamSig P, WithParam<P>::guardChecksSig/4 guardChecks> {
1000+
private predicate guardChecksAdjTypes(
1001+
Guards::Guards::Guard g, Expr e, Guards::GuardValue gv, P param
1002+
) {
1003+
guardChecks(g, e, gv, param)
1004+
}
1005+
1006+
private predicate guardChecksWithWrappers(
1007+
DataFlowIntegrationInput::Guard g, Definition def, Guards::GuardValue val, P param
1008+
) {
1009+
Guards::Guards::ParameterizedValidationWrapper<P, guardChecksAdjTypes/4>::guardChecksDef(g,
1010+
def, val, param)
1011+
}
1012+
1013+
private Node getABarrierNodeImpl(P param) {
1014+
result =
1015+
DataFlowIntegrationImpl::BarrierGuardDefWithState<P, guardChecksWithWrappers/4>::getABarrierNode(param)
1016+
}
1017+
1018+
predicate getABarrierNode = getABarrierNodeImpl/1;
1019+
}
9901020
}
9911021
}
9921022

0 commit comments

Comments
 (0)