Skip to content

Commit bd47dcc

Browse files
Jami CogswellJami Cogswell
authored andcommitted
Java: check first arg for taint
1 parent e8724ab commit bd47dcc

File tree

1 file changed

+23
-2
lines changed

1 file changed

+23
-2
lines changed

java/ql/lib/semmle/code/java/security/PathSanitizer.qll

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,21 @@ private predicate maybeNull(Expr expr) {
357357
)
358358
}
359359

360+
/** A taint-tracking configuration for reasoning about tainted arguments. */
361+
private module TaintedArgConfig implements DataFlow::ConfigSig {
362+
predicate isSource(DataFlow::Node src) {
363+
src instanceof ActiveThreatModelSource or
364+
src instanceof ApiSourceNode or
365+
// for InlineFlowTest
366+
src.asExpr().(MethodCall).getMethod().getName() = "source"
367+
}
368+
369+
predicate isSink(DataFlow::Node sink) { exists(Call call | sink.asExpr() = call.getAnArgument()) }
370+
}
371+
372+
/** Tracks taint flow to any argument. */
373+
private module TaintedArgFlow = TaintTracking::Global<TaintedArgConfig>;
374+
360375
/** Holds if `g` is a guard that checks for `..` components. */
361376
private predicate pathTraversalGuard(Guard g, Expr e, boolean branch) {
362377
// Local taint-flow is used here to handle cases where the validated expression comes from the
@@ -370,9 +385,12 @@ private predicate pathTraversalGuard(Guard g, Expr e, boolean branch) {
370385
}
371386

372387
/**
373-
* A sanitizer that considers the second argument to a `File` constructor safe
374-
* if it is checked for `..` components (`PathTraversalGuard`) or if any internal
388+
* A sanitizer that considers a `File` constructor safe if its second argument
389+
* is checked for `..` components (`PathTraversalGuard`) or if any internal
375390
* `..` components are removed from it (`PathNormalizeSanitizer`).
391+
*
392+
* This also requires a check to ensure that the first argument of the
393+
* `File` constructor is not tainted.
376394
*/
377395
private class FileConstructorSanitizer extends PathInjectionSanitizer {
378396
FileConstructorSanitizer() {
@@ -382,6 +400,9 @@ private class FileConstructorSanitizer extends PathInjectionSanitizer {
382400
// `java.io.File` documentation states that such cases are
383401
// treated as if invoking the single-argument `File` constructor.
384402
not maybeNull(constrCall.getArgument(0)) and
403+
// Since we are sanitizing the constructor call, we need to check
404+
// that the parent argument is not tainted.
405+
not TaintedArgFlow::flowToExpr(constrCall.getArgument(0)) and
385406
arg = constrCall.getArgument(1) and
386407
(
387408
arg = DataFlow::BarrierGuard<pathTraversalGuard/3>::getABarrierNode().asExpr() or

0 commit comments

Comments
 (0)