Skip to content

Commit 936f0c6

Browse files
committed
Address review comments on path-injection[read] sub-kind
- shared/mad/codeql/mad/ModelValidation.qll: shorten the comment for `path-injection[%]` to `// Java-only currently`, matching the style of other language-scoped entries and dropping API examples and the java/zipslip reference. - java/ql/lib/semmle/code/java/security/ZipSlipQuery.qll: replace the `File.exists` example in the QLDoc with `FileReader`, since `File.exists` is still labelled plain `path-injection`, not `path-injection[read]`.
1 parent 90741b1 commit 936f0c6

2 files changed

Lines changed: 3 additions & 6 deletions

File tree

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ module ZipSlipFlow = TaintTracking::Global<ZipSlipConfig>;
5555
*
5656
* This deliberately selects only the `path-injection` sink kind and excludes
5757
* `path-injection[read]`: Zip Slip is an archive-extraction vulnerability, so
58-
* read-only path sinks (e.g. `ClassLoader.getResource`, `FileInputStream`,
59-
* `File.exists`) are outside the threat model.
58+
* read-only path sinks (for example `ClassLoader.getResource`,
59+
* `FileInputStream`, and `FileReader`) are outside the threat model.
6060
*/
6161
private class FileCreationSink extends DataFlow::Node {
6262
FileCreationSink() { sinkNode(this, "path-injection") }

shared/mad/codeql/mad/ModelValidation.qll

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,7 @@ module KindValidation<KindValidationConfigSig Config> {
5454
this.matches([
5555
// shared
5656
"credentials-%", "encryption-%", "qltest%", "test-%", "regex-use%",
57-
// shared: path-injection[read] identifies sinks that only read from a path
58-
// (e.g. ClassLoader.getResource, FileInputStream, File.exists). Queries such
59-
// as java/zipslip that only care about write/extraction deliberately exclude
60-
// this sub-kind.
57+
// Java-only currently
6158
"path-injection[%]",
6259
// Swift-only currently, but may be shared in the future
6360
"%string-%length", "weak-hash-input-%",

0 commit comments

Comments
 (0)