Skip to content

Commit 7285a8e

Browse files
authored
Merge pull request #20986 from aschackmull/java/mad-barriers
Java: Support for MaD barriers and barrier guards.
2 parents 8fccc34 + 4066c0d commit 7285a8e

File tree

24 files changed

+469
-128
lines changed

24 files changed

+469
-128
lines changed

cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,19 @@ module SourceSinkInterpretationInput implements
148148
)
149149
}
150150

151+
predicate barrierElement(
152+
Element n, string output, string kind, Public::Provenance provenance, string model
153+
) {
154+
none()
155+
}
156+
157+
predicate barrierGuardElement(
158+
Element n, string input, Public::AcceptingValue acceptingvalue, string kind,
159+
Public::Provenance provenance, string model
160+
) {
161+
none()
162+
}
163+
151164
private newtype TInterpretNode =
152165
TElement_(Element n) or
153166
TNode_(Node n)

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,19 @@ module SourceSinkInterpretationInput implements
235235
)
236236
}
237237

238+
predicate barrierElement(
239+
Element n, string output, string kind, Public::Provenance provenance, string model
240+
) {
241+
none()
242+
}
243+
244+
predicate barrierGuardElement(
245+
Element n, string input, Public::AcceptingValue acceptingvalue, string kind,
246+
Public::Provenance provenance, string model
247+
) {
248+
none()
249+
}
250+
238251
class SourceOrSinkElement = Element;
239252

240253
private newtype TInterpretNode =

go/ql/lib/semmle/go/dataflow/internal/FlowSummaryImpl.qll

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,19 @@ module SourceSinkInterpretationInput implements
163163
)
164164
}
165165

166+
predicate barrierElement(
167+
Element n, string output, string kind, Public::Provenance provenance, string model
168+
) {
169+
none()
170+
}
171+
172+
predicate barrierGuardElement(
173+
Element n, string input, Public::AcceptingValue acceptingvalue, string kind,
174+
Public::Provenance provenance, string model
175+
) {
176+
none()
177+
}
178+
166179
// Note that due to embedding, which is currently implemented via some
167180
// Methods having multiple qualified names, a given Method is liable to have
168181
// more than one SourceOrSinkElement, one for each of the names it claims.

java/ql/lib/ext/empty.model.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ extensions:
1313
pack: codeql/java-all
1414
extensible: summaryModel
1515
data: []
16+
- addsTo:
17+
pack: codeql/java-all
18+
extensible: barrierModel
19+
data: []
20+
- addsTo:
21+
pack: codeql/java-all
22+
extensible: barrierGuardModel
23+
data: []
1624
- addsTo:
1725
pack: codeql/java-all
1826
extensible: neutralModel

java/ql/lib/ext/hudson.model.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ extensions:
5050
- ["hudson", "FilePath", False, "readToString", "", "", "ReturnValue", "file", "manual"]
5151
- ["hudson", "Plugin", True, "configure", "", "", "Parameter", "remote", "manual"]
5252
- ["hudson", "Plugin", True, "newInstance", "", "", "Parameter", "remote", "manual"]
53+
- addsTo:
54+
pack: codeql/java-all
55+
extensible: barrierModel
56+
data:
57+
- ["hudson", "Util", True, "escape", "(String)", "", "ReturnValue", "html-injection", "manual"]
58+
# Not including xmlEscape because it only accounts for >, <, and &. It does not account for ", or ', which makes it an incomplete XSS sanitizer.
5359
- addsTo:
5460
pack: codeql/java-all
5561
extensible: summaryModel

java/ql/lib/ext/java.io.model.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,8 @@ extensions:
162162
extensible: sourceModel
163163
data:
164164
- ["java.io", "FileInputStream", True, "FileInputStream", "", "", "Argument[this]", "file", "manual"]
165+
- addsTo:
166+
pack: codeql/java-all
167+
extensible: barrierModel
168+
data:
169+
- ["java.io", "File", True, "getName", "()", "", "ReturnValue", "path-injection", "manual"]

java/ql/lib/ext/java.net.model.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ extensions:
3434
- ["java.net", "URLClassLoader", False, "URLClassLoader", "(URL[],ClassLoader)", "", "Argument[0]", "request-forgery", "manual"]
3535
- ["java.net", "URLClassLoader", False, "URLClassLoader", "(URL[])", "", "Argument[0]", "request-forgery", "manual"]
3636
- ["java.net", "PasswordAuthentication", False, "PasswordAuthentication", "(String,char[])", "", "Argument[0]", "credentials-username", "hq-generated"]
37+
- addsTo:
38+
pack: codeql/java-all
39+
extensible: barrierGuardModel
40+
data:
41+
- ["java.net", "URI", True, "isAbsolute", "()", "", "Argument[this]", "false", "request-forgery", "manual"]
3742
- addsTo:
3843
pack: codeql/java-all
3944
extensible: summaryModel

java/ql/lib/ext/java.util.regex.model.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ extensions:
1212
- ["java.util.regex", "Pattern", False, "split", "(CharSequence)", "", "Argument[this]", "regex-use[0]", "manual"]
1313
- ["java.util.regex", "Pattern", False, "split", "(CharSequence,int)", "", "Argument[this]", "regex-use[0]", "manual"]
1414
- ["java.util.regex", "Pattern", False, "splitAsStream", "(CharSequence)", "", "Argument[this]", "regex-use[0]", "manual"]
15+
- addsTo:
16+
pack: codeql/java-all
17+
extensible: barrierModel
18+
data:
19+
- ["java.util.regex", "Pattern", False, "quote", "(String)", "", "ReturnValue", "regex-use", "manual"]
1520
- addsTo:
1621
pack: codeql/java-all
1722
extensible: summaryModel
Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,42 @@
11
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: barrierGuardModel
5+
data:
6+
- ["org.owasp.esapi", "Validator", true, "isValidCreditCard", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
7+
- ["org.owasp.esapi", "Validator", true, "isValidDate", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
8+
- ["org.owasp.esapi", "Validator", true, "isValidDirectoryPath", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
9+
- ["org.owasp.esapi", "Validator", true, "isValidDouble", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
10+
- ["org.owasp.esapi", "Validator", true, "isValidFileContent", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
11+
- ["org.owasp.esapi", "Validator", true, "isValidFileName", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
12+
- ["org.owasp.esapi", "Validator", true, "isValidInput", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
13+
- ["org.owasp.esapi", "Validator", true, "isValidInteger", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
14+
- ["org.owasp.esapi", "Validator", true, "isValidListItem", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
15+
- ["org.owasp.esapi", "Validator", true, "isValidNumber", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
16+
- ["org.owasp.esapi", "Validator", true, "isValidPrintable", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
17+
- ["org.owasp.esapi", "Validator", true, "isValidRedirectLocation", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
18+
- ["org.owasp.esapi", "Validator", true, "isValidSafeHTML", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
19+
- ["org.owasp.esapi", "Validator", true, "isValidURI", "", "", "Argument[1]", "true", "trust-boundary-violation", "manual"]
20+
- addsTo:
21+
pack: codeql/java-all
22+
extensible: barrierModel
23+
data:
24+
- ["org.owasp.esapi", "Validator", true, "getValidCreditCard", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
25+
- ["org.owasp.esapi", "Validator", true, "getValidDate", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
26+
- ["org.owasp.esapi", "Validator", true, "getValidDirectoryPath", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
27+
- ["org.owasp.esapi", "Validator", true, "getValidDouble", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
28+
- ["org.owasp.esapi", "Validator", true, "getValidFileContent", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
29+
- ["org.owasp.esapi", "Validator", true, "getValidFileName", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
30+
- ["org.owasp.esapi", "Validator", true, "getValidInput", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
31+
- ["org.owasp.esapi", "Validator", true, "getValidInteger", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
32+
- ["org.owasp.esapi", "Validator", true, "getValidListItem", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
33+
- ["org.owasp.esapi", "Validator", true, "getValidNumber", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
34+
- ["org.owasp.esapi", "Validator", true, "getValidPrintable", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
35+
- ["org.owasp.esapi", "Validator", true, "getValidRedirectLocation", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
36+
- ["org.owasp.esapi", "Validator", true, "getValidSafeHTML", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
37+
- ["org.owasp.esapi", "Validator", true, "getValidURI", "", "", "ReturnValue", "trust-boundary-violation", "manual"]
238
- addsTo:
339
pack: codeql/java-all
440
extensible: summaryModel
541
data:
6-
- ["org.owasp.esapi", "Encoder", true, "encodeForHTML", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
42+
- ["org.owasp.esapi", "Encoder", true, "encodeForHTML", "(String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 105 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ module;
9191

9292
import java
9393
private import semmle.code.java.dataflow.DataFlow::DataFlow
94+
private import semmle.code.java.controlflow.Guards
9495
private import FlowSummary as FlowSummary
9596
private import internal.DataFlowPrivate
9697
private import internal.FlowSummaryImpl
@@ -174,6 +175,24 @@ predicate sinkModel(
174175
)
175176
}
176177

178+
/** Holds if a barrier model exists for the given parameters. */
179+
predicate barrierModel(
180+
string package, string type, boolean subtypes, string name, string signature, string ext,
181+
string output, string kind, string provenance, QlBuiltins::ExtensionId madId
182+
) {
183+
Extensions::barrierModel(package, type, subtypes, name, signature, ext, output, kind, provenance,
184+
madId)
185+
}
186+
187+
/** Holds if a barrier guard model exists for the given parameters. */
188+
predicate barrierGuardModel(
189+
string package, string type, boolean subtypes, string name, string signature, string ext,
190+
string input, string acceptingvalue, string kind, string provenance, QlBuiltins::ExtensionId madId
191+
) {
192+
Extensions::barrierGuardModel(package, type, subtypes, name, signature, ext, input,
193+
acceptingvalue, kind, provenance, madId)
194+
}
195+
177196
/** Holds if a summary model exists for the given parameters. */
178197
predicate summaryModel(
179198
string package, string type, boolean subtypes, string name, string signature, string ext,
@@ -234,6 +253,7 @@ predicate interpretModelForTest(QlBuiltins::ExtensionId madId, string model) {
234253
"Summary: " + package + "; " + type + "; " + subtypes + "; " + name + "; " + signature + "; " +
235254
ext + "; " + input + "; " + output + "; " + kind + "; " + provenance
236255
)
256+
//TODO: possibly barrier models?
237257
}
238258

239259
/** Holds if a neutral model exists for the given parameters. */
@@ -292,6 +312,7 @@ predicate modelCoverage(string package, int pkgs, string kind, string part, int
292312
summaryModel(subpkg, type, subtypes, name, signature, ext, input, output, kind, provenance,
293313
_)
294314
)
315+
// TODO: possibly barrier models?
295316
)
296317
}
297318

@@ -303,7 +324,9 @@ module ModelValidation {
303324
summaryModel(_, _, _, _, _, _, path, _, _, _, _) or
304325
summaryModel(_, _, _, _, _, _, _, path, _, _, _) or
305326
sinkModel(_, _, _, _, _, _, path, _, _, _) or
306-
sourceModel(_, _, _, _, _, _, path, _, _, _)
327+
sourceModel(_, _, _, _, _, _, path, _, _, _) or
328+
barrierModel(_, _, _, _, _, _, path, _, _, _) or
329+
barrierGuardModel(_, _, _, _, _, _, path, _, _, _, _)
307330
}
308331

309332
private module MkAccessPath = AccessPathSyntax::AccessPath<getRelevantAccessPath/1>;
@@ -316,6 +339,8 @@ module ModelValidation {
316339
exists(string pred, AccessPath input, AccessPathToken part |
317340
sinkModel(_, _, _, _, _, _, input, _, _, _) and pred = "sink"
318341
or
342+
barrierGuardModel(_, _, _, _, _, _, input, _, _, _, _) and pred = "barrier guard"
343+
or
319344
summaryModel(_, _, _, _, _, _, input, _, _, _, _) and pred = "summary"
320345
|
321346
(
@@ -338,6 +363,8 @@ module ModelValidation {
338363
exists(string pred, AccessPath output, AccessPathToken part |
339364
sourceModel(_, _, _, _, _, _, output, _, _, _) and pred = "source"
340365
or
366+
barrierModel(_, _, _, _, _, _, output, _, _, _) and pred = "barrier"
367+
or
341368
summaryModel(_, _, _, _, _, _, _, output, _, _, _) and pred = "summary"
342369
|
343370
(
@@ -355,7 +382,13 @@ module ModelValidation {
355382
private module KindValConfig implements SharedModelVal::KindValidationConfigSig {
356383
predicate summaryKind(string kind) { summaryModel(_, _, _, _, _, _, _, _, kind, _, _) }
357384

358-
predicate sinkKind(string kind) { sinkModel(_, _, _, _, _, _, _, kind, _, _) }
385+
predicate sinkKind(string kind) {
386+
sinkModel(_, _, _, _, _, _, _, kind, _, _)
387+
or
388+
barrierModel(_, _, _, _, _, _, _, kind, _, _)
389+
or
390+
barrierGuardModel(_, _, _, _, _, _, _, _, kind, _, _)
391+
}
359392

360393
predicate sourceKind(string kind) { sourceModel(_, _, _, _, _, _, _, kind, _, _) }
361394

@@ -373,6 +406,11 @@ module ModelValidation {
373406
or
374407
sinkModel(package, type, _, name, signature, ext, _, _, provenance, _) and pred = "sink"
375408
or
409+
barrierModel(package, type, _, name, signature, ext, _, _, provenance, _) and pred = "barrier"
410+
or
411+
barrierGuardModel(package, type, _, name, signature, ext, _, _, _, provenance, _) and
412+
pred = "barrier guard"
413+
or
376414
summaryModel(package, type, _, name, signature, ext, _, _, _, provenance, _) and
377415
pred = "summary"
378416
or
@@ -398,6 +436,14 @@ module ModelValidation {
398436
invalidProvenance(provenance) and
399437
result = "Unrecognized provenance description \"" + provenance + "\" in " + pred + " model."
400438
)
439+
or
440+
exists(string acceptingvalue |
441+
barrierGuardModel(_, _, _, _, _, _, _, acceptingvalue, _, _, _) and
442+
invalidAcceptingValue(acceptingvalue) and
443+
result =
444+
"Unrecognized accepting value description \"" + acceptingvalue +
445+
"\" in barrier guard model."
446+
)
401447
}
402448

403449
/** Holds if some row in a MaD flow model appears to contain typos. */
@@ -418,6 +464,10 @@ private predicate elementSpec(
418464
or
419465
sinkModel(package, type, subtypes, name, signature, ext, _, _, _, _)
420466
or
467+
barrierModel(package, type, subtypes, name, signature, ext, _, _, _, _)
468+
or
469+
barrierGuardModel(package, type, subtypes, name, signature, ext, _, _, _, _, _)
470+
or
421471
summaryModel(package, type, subtypes, name, signature, ext, _, _, _, _, _)
422472
or
423473
neutralModel(package, type, name, signature, _, _) and ext = "" and subtypes = true
@@ -578,6 +628,53 @@ private module Cached {
578628
isSinkNode(n, kind, model) and n.asNode() = node
579629
)
580630
}
631+
632+
private newtype TKindModelPair =
633+
TMkPair(string kind, string model) { isBarrierGuardNode(_, _, kind, model) }
634+
635+
private GuardValue convertAcceptingValue(AcceptingValue av) {
636+
av.isTrue() and result.asBooleanValue() = true
637+
or
638+
av.isFalse() and result.asBooleanValue() = false
639+
or
640+
av.isNoException() and result.getDualValue().isThrowsException()
641+
or
642+
av.isZero() and result.asIntValue() = 0
643+
or
644+
av.isNotZero() and result.getDualValue().asIntValue() = 0
645+
or
646+
av.isNull() and result.isNullValue()
647+
or
648+
av.isNotNull() and result.isNonNullValue()
649+
}
650+
651+
private predicate barrierGuardChecks(Guard g, Expr e, GuardValue gv, TKindModelPair kmp) {
652+
exists(
653+
SourceSinkInterpretationInput::InterpretNode n, AcceptingValue acceptingvalue, string kind,
654+
string model
655+
|
656+
isBarrierGuardNode(n, acceptingvalue, kind, model) and
657+
n.asNode().asExpr() = e and
658+
kmp = TMkPair(kind, model) and
659+
gv = convertAcceptingValue(acceptingvalue)
660+
|
661+
g.(Call).getAnArgument() = e or g.(MethodCall).getQualifier() = e
662+
)
663+
}
664+
665+
/**
666+
* Holds if `node` is specified as a barrier with the given kind in a MaD flow
667+
* model.
668+
*/
669+
cached
670+
predicate barrierNode(Node node, string kind, string model) {
671+
exists(SourceSinkInterpretationInput::InterpretNode n |
672+
isBarrierNode(n, kind, model) and n.asNode() = node
673+
)
674+
or
675+
ParameterizedBarrierGuard<TKindModelPair, barrierGuardChecks/4>::getABarrierNode(TMkPair(kind,
676+
model)) = node
677+
}
581678
}
582679

583680
import Cached
@@ -594,6 +691,12 @@ predicate sourceNode(Node node, string kind) { sourceNode(node, kind, _) }
594691
*/
595692
predicate sinkNode(Node node, string kind) { sinkNode(node, kind, _) }
596693

694+
/**
695+
* Holds if `node` is specified as a barrier with the given kind in a MaD flow
696+
* model.
697+
*/
698+
predicate barrierNode(Node node, string kind) { barrierNode(node, kind, _) }
699+
597700
// adapter class for converting Mad summaries to `SummarizedCallable`s
598701
private class SummarizedCallableAdapter extends SummarizedCallable {
599702
SummarizedCallableAdapter() { summaryElement(this, _, _, _, _, _, _) }

0 commit comments

Comments
 (0)