Move tainted path ad-hoc guard back.

This commit is contained in:
intrigus
2020-07-19 00:19:29 +02:00
parent 33526f61a8
commit f94055fa2c
4 changed files with 38 additions and 28 deletions

View File

@@ -16,6 +16,7 @@ import java
import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.PathCreation import semmle.code.java.security.PathCreation
import DataFlow::PathGraph import DataFlow::PathGraph
import TaintedPathCommon
class ContainsDotDotSanitizer extends DataFlow::BarrierGuard { class ContainsDotDotSanitizer extends DataFlow::BarrierGuard {
ContainsDotDotSanitizer() { ContainsDotDotSanitizer() {

View File

@@ -0,0 +1,33 @@
/**
* Models a very basic guard for the tainted path queries.
*/
import java
import semmle.code.java.controlflow.Guards
import semmle.code.java.security.PathCreation
private predicate inWeakCheck(Expr e) {
// None of these are sufficient to guarantee that a string is safe.
exists(MethodAccess m, Method def | m.getQualifier() = e and m.getMethod() = def |
def.getName() = "startsWith" or
def.getName() = "endsWith" or
def.getName() = "isEmpty" or
def.getName() = "equals"
)
or
// Checking against `null` has no bearing on path traversal.
exists(EqualityTest b | b.getAnOperand() = e | b.getAnOperand() instanceof NullLiteral)
}
// Ignore cases where the variable has been checked somehow,
// but allow some particularly obviously bad cases.
predicate guarded(VarAccess e) {
exists(PathCreation p | e = p.getAnInput()) and
exists(ConditionBlock cb, Expr c |
cb.getCondition().getAChildExpr*() = c and
c = e.getVariable().getAnAccess() and
cb.controls(e.getBasicBlock(), true) and
// Disallow a few obviously bad checks.
not inWeakCheck(c)
)
}

View File

@@ -16,13 +16,16 @@ import java
import semmle.code.java.dataflow.FlowSources import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.PathCreation import semmle.code.java.security.PathCreation
import DataFlow::PathGraph import DataFlow::PathGraph
import TaintedPathCommon
class TaintedPathLocalConfig extends TaintTracking::Configuration { class TaintedPathLocalConfig extends TaintTracking::Configuration {
TaintedPathLocalConfig() { this = "TaintedPathLocalConfig" } TaintedPathLocalConfig() { this = "TaintedPathLocalConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput } override predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
override predicate isSink(DataFlow::Node sink) { sink.asExpr() = any(PathCreation p).getAnInput() } override predicate isSink(DataFlow::Node sink) {
sink.asExpr() = any(PathCreation p).getAnInput()
}
} }
from from

View File

@@ -3,7 +3,6 @@
*/ */
import java import java
import semmle.code.java.controlflow.Guards
/** Models the creation of a path. */ /** Models the creation of a path. */
abstract class PathCreation extends Expr { abstract class PathCreation extends Expr {
@@ -140,29 +139,3 @@ private class FileOutputStreamCreation extends PathCreation, ClassInstanceExpr {
result.getType() instanceof TypeString result.getType() instanceof TypeString
} }
} }
private predicate inWeakCheck(Expr e) {
// None of these are sufficient to guarantee that a string is safe.
exists(MethodAccess m, Method def | m.getQualifier() = e and m.getMethod() = def |
def.getName() = "startsWith" or
def.getName() = "endsWith" or
def.getName() = "isEmpty" or
def.getName() = "equals"
)
or
// Checking against `null` has no bearing on path traversal.
exists(EqualityTest b | b.getAnOperand() = e | b.getAnOperand() instanceof NullLiteral)
}
// Ignore cases where the variable has been checked somehow,
// but allow some particularly obviously bad cases.
predicate guarded(VarAccess e) {
exists(PathCreation p | e = p.getAnInput()) and
exists(ConditionBlock cb, Expr c |
cb.getCondition().getAChildExpr*() = c and
c = e.getVariable().getAnAccess() and
cb.controls(e.getBasicBlock(), true) and
// Disallow a few obviously bad checks.
not inWeakCheck(c)
)
}