mirror of
https://github.com/github/codeql.git
synced 2026-05-14 11:19:27 +02:00
c336a1595d
Introduce a new Models-as-Data sink sub-kind path-injection[read] for models that only read from or inspect a path. The general java/path-injection query and its PathInjectionSanitizer barrier continue to consider both path-injection and path-injection[read] sinks, so no alerts are lost. The java/zipslip query deliberately selects only path-injection sinks, since read-only accesses such as ClassLoader.getResource or FileInputStream are outside the archive extraction threat model. Addresses https://github.com/github/codeql/issues/21606 along the lines proposed on the issue thread: prefer path-injection[read] over a [create] sub-kind so that miscategorizing a sink causes a false positive (easy to spot) rather than a false negative. - shared/mad/codeql/mad/ModelValidation.qll: allow path-injection[...] as a valid sink kind. - java/ql/lib/ext/*.model.yml: relabel the models that PR #12916 migrated from the historical read-file kind (plus the newer ClassLoader resource-lookup variants that share the same read-only semantics). - java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll and PathSanitizer.qll: select both path-injection and path-injection[read] sinks/barriers. - java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll: keep only path-injection, with a comment explaining why path-injection[read] is excluded. - java/ql/test/query-tests/security/CWE-022/semmle/tests/ZipTest.java: add m7 regression covering the Dubbo-style classpath lookup from issue #21606 and assert no alert is produced. - Update TaintedPath.expected for the renamed kinds in the models list. - Add change-notes under java/ql/lib/change-notes and java/ql/src/change-notes.
106 lines
3.6 KiB
Plaintext
106 lines
3.6 KiB
Plaintext
/** Provides dataflow configurations for tainted path queries. */
|
|
|
|
import java
|
|
import semmle.code.java.frameworks.Networking
|
|
import semmle.code.java.dataflow.DataFlow
|
|
import semmle.code.java.dataflow.FlowSources
|
|
private import semmle.code.java.dataflow.ExternalFlow
|
|
import semmle.code.java.security.PathSanitizer
|
|
private import semmle.code.java.security.Sanitizers
|
|
|
|
/** A sink for tainted path flow configurations. */
|
|
abstract class TaintedPathSink extends DataFlow::Node { }
|
|
|
|
private class DefaultTaintedPathSink extends TaintedPathSink {
|
|
DefaultTaintedPathSink() { sinkNode(this, ["path-injection", "path-injection[read]"]) }
|
|
}
|
|
|
|
/**
|
|
* A unit class for adding additional taint steps.
|
|
*
|
|
* Extend this class to add additional taint steps that should apply to tainted path flow configurations.
|
|
*/
|
|
class TaintedPathAdditionalTaintStep extends Unit {
|
|
abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
|
|
}
|
|
|
|
private class DefaultTaintedPathAdditionalTaintStep extends TaintedPathAdditionalTaintStep {
|
|
override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
|
|
exists(Argument a |
|
|
a = n1.asExpr() and
|
|
a.getCall() = n2.asExpr() and
|
|
a = any(TaintPreservingUriCtorParam tpp).getAnArgument()
|
|
)
|
|
}
|
|
}
|
|
|
|
private class TaintPreservingUriCtorParam extends Parameter {
|
|
TaintPreservingUriCtorParam() {
|
|
exists(Constructor ctor, int idx, int nParams |
|
|
ctor.getDeclaringType() instanceof TypeUri and
|
|
this = ctor.getParameter(idx) and
|
|
nParams = ctor.getNumberOfParameters()
|
|
|
|
|
// URI(String scheme, String ssp, String fragment)
|
|
idx = 1 and nParams = 3
|
|
or
|
|
// URI(String scheme, String host, String path, String fragment)
|
|
idx = [1, 2] and nParams = 4
|
|
or
|
|
// URI(String scheme, String authority, String path, String query, String fragment)
|
|
idx = 2 and nParams = 5
|
|
or
|
|
// URI(String scheme, String userInfo, String host, int port, String path, String query, String fragment)
|
|
idx = 4 and nParams = 7
|
|
)
|
|
}
|
|
}
|
|
|
|
/**
|
|
* A taint-tracking configuration for tracking flow from remote sources to the creation of a path.
|
|
*/
|
|
module TaintedPathConfig implements DataFlow::ConfigSig {
|
|
predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
|
|
|
|
predicate isSink(DataFlow::Node sink) { sink instanceof TaintedPathSink }
|
|
|
|
predicate isBarrier(DataFlow::Node sanitizer) {
|
|
sanitizer instanceof SimpleTypeSanitizer or
|
|
sanitizer instanceof PathInjectionSanitizer
|
|
}
|
|
|
|
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
|
|
any(TaintedPathAdditionalTaintStep s).step(n1, n2)
|
|
}
|
|
|
|
predicate observeDiffInformedIncrementalMode() { any() }
|
|
}
|
|
|
|
/** Tracks flow from remote sources to the creation of a path. */
|
|
module TaintedPathFlow = TaintTracking::Global<TaintedPathConfig>;
|
|
|
|
/**
|
|
* A taint-tracking configuration for tracking flow from local user input to the creation of a path.
|
|
*/
|
|
deprecated module TaintedPathLocalConfig implements DataFlow::ConfigSig {
|
|
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
|
|
|
|
predicate isSink(DataFlow::Node sink) { sink instanceof TaintedPathSink }
|
|
|
|
predicate isBarrier(DataFlow::Node sanitizer) {
|
|
sanitizer instanceof SimpleTypeSanitizer or
|
|
sanitizer instanceof PathInjectionSanitizer
|
|
}
|
|
|
|
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
|
|
any(TaintedPathAdditionalTaintStep s).step(n1, n2)
|
|
}
|
|
}
|
|
|
|
/**
|
|
* DEPRECATED: Use `TaintedPathFlow` instead and configure threat model sources to include `local`.
|
|
*
|
|
* Tracks flow from local user input to the creation of a path.
|
|
*/
|
|
deprecated module TaintedPathLocalFlow = TaintTracking::Global<TaintedPathLocalConfig>;
|