Skip to content

Commit c336a15

Browse files
committed
Java: split read-only path sinks into path-injection[read]
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 #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.
1 parent 7f2a13b commit c336a15

29 files changed

Lines changed: 134 additions & 102 deletions
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Introduced a new sink kind `path-injection[read]` for Models-as-Data rows that only read from a path (such as `ClassLoader.getResource`, `FileInputStream`, `FileReader`, `Files.readAllBytes`, and related APIs). The general `java/path-injection` query continues to consider both `path-injection` and `path-injection[read]` sinks.

java/ql/lib/ext/com.google.common.io.model.yml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,12 @@ extensions:
55
data:
66
- ["com.google.common.io", "Files", False, "asByteSink", "(File,FileWriteMode[])", "", "Argument[0]", "path-injection", "ai-manual"]
77
- ["com.google.common.io", "Files", False, "asCharSink", "(File,Charset,FileWriteMode[])", "", "Argument[0]", "path-injection", "ai-manual"]
8-
- ["com.google.common.io", "Files", False, "asCharSource", "(File,Charset)", "", "Argument[0]", "path-injection", "ai-manual"]
9-
- ["com.google.common.io", "Files", False, "copy", "(File,OutputStream)", "", "Argument[0]", "path-injection", "ai-manual"]
8+
- ["com.google.common.io", "Files", False, "asCharSource", "(File,Charset)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
9+
- ["com.google.common.io", "Files", False, "copy", "(File,OutputStream)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
1010
- ["com.google.common.io", "Files", False, "newWriter", "(File,Charset)", "", "Argument[0]", "path-injection", "ai-manual"]
11-
- ["com.google.common.io", "Files", False, "readLines", "(File,Charset)", "", "Argument[0]", "path-injection", "ai-manual"]
12-
- ["com.google.common.io", "Files", False, "toByteArray", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
13-
- ["com.google.common.io", "Files", False, "toString", "(File,Charset)", "", "Argument[0]", "path-injection", "ai-manual"]
11+
- ["com.google.common.io", "Files", False, "readLines", "(File,Charset)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
12+
- ["com.google.common.io", "Files", False, "toByteArray", "(File)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
13+
- ["com.google.common.io", "Files", False, "toString", "(File,Charset)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
1414
- ["com.google.common.io", "Files", False, "write", "(byte[],File)", "", "Argument[0]", "file-content-store", "ai-manual"]
1515
- ["com.google.common.io", "Files", False, "write", "(byte[],File)", "", "Argument[1]", "path-injection", "manual"]
1616
- addsTo:

java/ql/lib/ext/com.thoughtworks.xstream.model.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ extensions:
33
pack: codeql/java-all
44
extensible: sinkModel
55
data:
6-
- ["com.thoughtworks.xstream", "XStream", True, "fromXML", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
6+
- ["com.thoughtworks.xstream", "XStream", True, "fromXML", "(File)", "", "Argument[0]", "path-injection[read]", "ai-manual"]

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ extensions:
55
data:
66
- ["hudson.model", "DownloadService", True, "loadJSON", "(URL)", "", "Argument[0]", "request-forgery", "ai-manual"]
77
- ["hudson.model", "DownloadService", True, "loadJSONHTML", "(URL)", "", "Argument[0]", "request-forgery", "ai-manual"]
8-
- ["hudson.model", "DirectoryBrowserSupport", False, "DirectoryBrowserSupport", "(ModelObject,FilePath,String,String,boolean)", "", "Argument[1]", "path-injection", "ai-manual"]
9-
- ["hudson.model", "Items", True, "load", "(ItemGroup,File)", "", "Argument[1]", "path-injection", "ai-manual"]
8+
- ["hudson.model", "DirectoryBrowserSupport", False, "DirectoryBrowserSupport", "(ModelObject,FilePath,String,String,boolean)", "", "Argument[1]", "path-injection[read]", "ai-manual"]
9+
- ["hudson.model", "Items", True, "load", "(ItemGroup,File)", "", "Argument[1]", "path-injection[read]", "ai-manual"]
1010
- ["hudson.model", "UpdateCenter$UpdateCenterConfiguration", True, "download", "(UpdateCenter$DownloadJob,URL)", "", "Argument[1]", "request-forgery", "ai-manual"]
1111
- ["hudson.model", "UpdateCenter$UpdateCenterConfiguration", True, "install", "(UpdateCenter$DownloadJob,File,File)", "", "Argument[1]", "path-injection", "ai-manual"]
1212
- ["hudson.model", "UpdateCenter$UpdateCenterConfiguration", True, "install", "(UpdateCenter$DownloadJob,File,File)", "", "Argument[2]", "path-injection", "ai-manual"]

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,22 @@ extensions:
66
- ["hudson", "FilePath", False, "tar", "(OutputStream,String)", "", "Argument[0]", "path-injection", "ai-manual"]
77
- ["hudson", "FilePath", False, "unzipFrom", "(InputStream)", "", "Argument[0]", "path-injection", "ai-manual"]
88
- ["hudson", "FilePath", True, "copyFrom", "", "", "Argument[this]", "path-injection", "manual"]
9-
- ["hudson", "FilePath", True, "copyFrom", "(FilePath)", "", "Argument[0]", "path-injection", "manual"]
10-
- ["hudson", "FilePath", True, "copyFrom", "(URL)", "", "Argument[0]", "path-injection", "manual"]
11-
- ["hudson", "FilePath", True, "copyFrom", "(FileItem)", "", "Argument[0]", "path-injection", "ai-manual"]
9+
- ["hudson", "FilePath", True, "copyFrom", "(FilePath)", "", "Argument[0]", "path-injection[read]", "manual"]
10+
- ["hudson", "FilePath", True, "copyFrom", "(URL)", "", "Argument[0]", "path-injection[read]", "manual"]
11+
- ["hudson", "FilePath", True, "copyFrom", "(FileItem)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
1212
- ["hudson", "FilePath", True, "copyRecursiveTo", "", "", "Argument[this]", "path-injection", "ai-manual"]
1313
- ["hudson", "FilePath", True, "copyRecursiveTo", "(DirScanner,FilePath,String,FilePath$TarCompression)", "", "Argument[1]", "path-injection", "ai-manual"]
1414
- ["hudson", "FilePath", True, "copyRecursiveTo", "(DirScanner,FilePath,String)", "", "Argument[1]", "path-injection", "ai-manual"]
1515
- ["hudson", "FilePath", True, "copyRecursiveTo", "(String,FilePath)", "", "Argument[1]", "path-injection", "ai-manual"]
16-
- ["hudson", "FilePath", True, "copyRecursiveTo", "(String,String,FilePath)", "", "Argument[0]", "path-injection", "ai-manual"]
16+
- ["hudson", "FilePath", True, "copyRecursiveTo", "(String,String,FilePath)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
1717
- ["hudson", "FilePath", True, "copyRecursiveTo", "(String,String,FilePath)", "", "Argument[2]", "path-injection", "ai-manual"]
1818
- ["hudson", "FilePath", True, "copyTo", "", "", "Argument[this]", "path-injection", "manual"]
1919
- ["hudson", "FilePath", True, "copyTo", "(FilePath)", "", "Argument[0]", "path-injection", "ai-manual"]
2020
- ["hudson", "FilePath", True, "copyToWithPermission", "", "", "Argument[this]", "path-injection", "manual"]
2121
- ["hudson", "FilePath", True, "copyToWithPermission", "(FilePath)", "", "Argument[0]", "path-injection", "manual"]
2222
- ["hudson", "FilePath", True, "exists", "()", "", "Argument[this]", "path-injection", "manual"]
2323
- ["hudson", "FilePath", True, "installIfNecessaryFrom", "(URL,TaskListener,String)", "", "Argument[0]", "request-forgery", "ai-manual"]
24-
- ["hudson", "FilePath", True, "newInputStreamDenyingSymlinkAsNeeded", "(File,String,boolean)", "", "Argument[0]", "path-injection", "ai-manual"]
24+
- ["hudson", "FilePath", True, "newInputStreamDenyingSymlinkAsNeeded", "(File,String,boolean)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
2525
- ["hudson", "FilePath", True, "openInputStream", "(File,OpenOption[])", "", "Argument[0]", "path-injection", "manual"]
2626
- ["hudson", "FilePath", True, "read", "", "", "Argument[this]", "path-injection", "manual"]
2727
- ["hudson", "FilePath", True, "read", "(FilePath,OpenOption[])", "", "Argument[0]", "path-injection", "manual"]

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ extensions:
33
pack: codeql/java-all
44
extensible: sinkModel
55
data:
6-
- ["hudson.scm", "ChangeLogParser", True, "parse", "(AbstractBuild,File)", "", "Argument[1]", "path-injection", "ai-manual"]
7-
- ["hudson.scm", "ChangeLogParser", True, "parse", "(Run,RepositoryBrowser,File)", "", "Argument[2]", "path-injection", "ai-manual"]
6+
- ["hudson.scm", "ChangeLogParser", True, "parse", "(AbstractBuild,File)", "", "Argument[1]", "path-injection[read]", "ai-manual"]
7+
- ["hudson.scm", "ChangeLogParser", True, "parse", "(Run,RepositoryBrowser,File)", "", "Argument[2]", "path-injection[read]", "ai-manual"]
88
- ["hudson.scm", "SCM", True, "checkout", "(AbstractBuild,Launcher,FilePath,BuildListener,File)", "", "Argument[2]", "path-injection", "ai-manual"]
99
- ["hudson.scm", "SCM", True, "checkout", "(Run,Launcher,FilePath,TaskListener,File,SCMRevisionState)", "", "Argument[2]", "path-injection", "ai-manual"]
10-
- ["hudson.scm", "SCM", True, "compareRemoteRevisionWith", "(Job,Launcher,FilePath,TaskListener,SCMRevisionState)", "", "Argument[2]", "path-injection", "ai-manual"]
10+
- ["hudson.scm", "SCM", True, "compareRemoteRevisionWith", "(Job,Launcher,FilePath,TaskListener,SCMRevisionState)", "", "Argument[2]", "path-injection[read]", "ai-manual"]
1111
- addsTo:
1212
pack: codeql/java-all
1313
extensible: summaryModel

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,6 @@ extensions:
33
pack: codeql/java-all
44
extensible: sinkModel
55
data:
6-
- ["hudson.util.jna", "GNUCLibrary", True, "open", "(String,int)", "", "Argument[0]", "path-injection", "ai-manual"]
6+
- ["hudson.util.jna", "GNUCLibrary", True, "open", "(String,int)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
77
- ["hudson.util.jna", "Kernel32", True, "MoveFileExA", "(String,String,int)", "", "Argument[0]", "path-injection", "ai-manual"]
88
- ["hudson.util.jna", "Kernel32", True, "MoveFileExA", "(String,String,int)", "", "Argument[1]", "path-injection", "ai-manual"]

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,19 @@ extensions:
66
- ["hudson.util", "AtomicFileWriter", True, "AtomicFileWriter", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
77
- ["hudson.util", "AtomicFileWriter", True, "AtomicFileWriter", "(Path,Charset,boolean,boolean)", "", "Argument[0]", "path-injection", "ai-manual"]
88
- ["hudson.util", "AtomicFileWriter", True, "AtomicFileWriter", "(Path,Charset)", "", "Argument[0]", "path-injection", "ai-manual"]
9-
- ["hudson.util", "ClasspathBuilder", True, "add", "(FilePath)", "", "Argument[0]", "path-injection", "ai-manual"]
9+
- ["hudson.util", "ClasspathBuilder", True, "add", "(FilePath)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
1010
- ["hudson.util", "FormValidation", True, "errorWithMarkup", "", "", "Argument[0]", "html-injection", "manual"]
1111
- ["hudson.util", "FormValidation", True, "okWithMarkup", "", "", "Argument[0]", "html-injection", "manual"]
1212
- ["hudson.util", "FormValidation", True, "respond", "", "", "Argument[1]", "html-injection", "manual"]
1313
- ["hudson.util", "FormValidation", True, "warningWithMarkup", "", "", "Argument[0]", "html-injection", "manual"]
1414
- ["hudson.util", "IOUtils", True, "mkdirs", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
1515
- ["hudson.util", "StreamTaskListener", True, "StreamTaskListener", "(File,boolean,Charset)", "", "Argument[0]", "path-injection", "ai-manual"]
1616
- ["hudson.util", "TextFile", True, "delete", "()", "", "Argument[this]", "path-injection", "manual"]
17-
- ["hudson.util", "TextFile", True, "fastTail", "", "", "Argument[this]", "path-injection", "manual"]
18-
- ["hudson.util", "TextFile", True, "head", "", "", "Argument[this]", "path-injection", "manual"]
19-
- ["hudson.util", "TextFile", True, "lines", "()", "", "Argument[this]", "path-injection", "manual"]
20-
- ["hudson.util", "TextFile", True, "read", "()", "", "Argument[this]", "path-injection", "manual"]
21-
- ["hudson.util", "TextFile", True, "readTrim", "()", "", "Argument[this]", "path-injection", "manual"]
17+
- ["hudson.util", "TextFile", True, "fastTail", "", "", "Argument[this]", "path-injection[read]", "manual"]
18+
- ["hudson.util", "TextFile", True, "head", "", "", "Argument[this]", "path-injection[read]", "manual"]
19+
- ["hudson.util", "TextFile", True, "lines", "()", "", "Argument[this]", "path-injection[read]", "manual"]
20+
- ["hudson.util", "TextFile", True, "read", "()", "", "Argument[this]", "path-injection[read]", "manual"]
21+
- ["hudson.util", "TextFile", True, "readTrim", "()", "", "Argument[this]", "path-injection[read]", "manual"]
2222
- ["hudson.util", "TextFile", True, "write", "(String)", "", "Argument[this]", "path-injection", "manual"]
2323
- ["hudson.util", "TextFile", True, "write", "(String)", "", "Argument[0]", "file-content-store", "manual"]
2424
- ["hudson.util", "HttpResponses", True, "staticResource", "(File)", "", "Argument[0]", "path-injection", "manual"]

java/ql/lib/ext/io.netty.handler.codec.http.multipart.model.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ extensions:
33
pack: codeql/java-all
44
extensible: sinkModel
55
data:
6-
- ["io.netty.handler.codec.http.multipart", "HttpPostRequestEncoder", True, "addBodyFileUpload", "(String,File,String,boolean)", "", "Argument[1]", "path-injection", "ai-manual"]
6+
- ["io.netty.handler.codec.http.multipart", "HttpPostRequestEncoder", True, "addBodyFileUpload", "(String,File,String,boolean)", "", "Argument[1]", "path-injection[read]", "ai-manual"]
77
- addsTo:
88
pack: codeql/java-all
99
extensible: summaryModel

java/ql/lib/ext/io.netty.handler.ssl.model.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ extensions:
33
pack: codeql/java-all
44
extensible: sinkModel
55
data:
6-
- ["io.netty.handler.ssl", "OpenSslServerContext", False, "OpenSslServerContext", "(File,File)", "", "Argument[0]", "path-injection", "ai-manual"]
7-
- ["io.netty.handler.ssl", "SslContextBuilder", False, "forServer", "(File,File)", "", "Argument[0]", "path-injection", "ai-manual"]
6+
- ["io.netty.handler.ssl", "OpenSslServerContext", False, "OpenSslServerContext", "(File,File)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
7+
- ["io.netty.handler.ssl", "SslContextBuilder", False, "forServer", "(File,File)", "", "Argument[0]", "path-injection[read]", "ai-manual"]
88
- ["io.netty.handler.ssl", "SslContextBuilder", False, "trustManager", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
99
- ["io.netty.handler.ssl", "SslContextBuilder", False, "trustManager", "(InputStream)", "", "Argument[0]", "path-injection", "ai-manual"]

0 commit comments

Comments
 (0)