hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-19 22:14:01 +02:00
Code Issues Packages Projects Releases Wiki Activity

Add PathCreation.qll sinks to models-as-data

Browse Source 
The old PathCreation sinks can't be removed because doing so would cause alert wobble in the path injection queries. See their getReportingNode predicates.
This commit is contained in:
Tony Torralba
2023-05-19 16:42:08 +02:00
parent 84a7b3ca52
commit 527fe523a8
10 changed files with 36 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
java/ql/lib/change-notes/2023-05-19-path-injection-sinks-mad.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Path creation sinks modeled in `PathCreation.qll` have been added to the models-as-data sink kinds `create-file` and `read-file`.

5
java/ql/lib/ext/java.io.model.yml
View File

@@ -3,6 +3,10 @@ extensions:
pack: codeql/java-all
extensible: sinkModel
data:
- ["java.io", "File", False, "File", "(File,String)", "", "Argument[1]", "path-injection", "manual"] # old PathCreation
- ["java.io", "File", False, "File", "(String)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation
- ["java.io", "File", False, "File", "(String,String)", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation
- ["java.io", "File", False, "File", "(URI)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation
- ["java.io", "File", True, "createTempFile", "(String,String,File)", "", "Argument[2]", "path-injection", "ai-manual"]
- ["java.io", "File", True, "renameTo", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.io", "FileInputStream", True, "FileInputStream", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
@@ -11,6 +15,7 @@ extensions:
- ["java.io", "FileOutputStream", False, "write", "", "", "Argument[0]", "file-content-store", "manual"]
- ["java.io", "FileReader", True, "FileReader", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.io", "FileReader", True, "FileReader", "(String)", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.io", "FileReader", True, "FileReader", "(String,Charset)", "", "Argument[0]", "path-injection", "manual"]
- ["java.io", "FileSystem", True, "createDirectory", "(File)", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.io", "FileWriter", False, "FileWriter", "", "", "Argument[0]", "path-injection", "manual"]
- ["java.io", "PrintStream", False, "PrintStream", "(File)", "", "Argument[0]", "path-injection", "manual"]

13
java/ql/lib/ext/java.nio.file.model.yml
View File

@@ -3,11 +3,9 @@ extensions:
pack: codeql/java-all
extensible: sinkModel
data:
- ["java.nio.file", "Files", False, "copy", "(Path,OutputStream)", "", "Argument[0]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "copy", "(Path,Path,CopyOption[])", "", "Argument[0]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "copy", "(Path,Path,CopyOption[])", "", "Argument[1]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "copy", "", "", "Argument[0]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "copy", "(InputStream,Path,CopyOption[])", "", "Argument[0]", "file-content-store", "manual"]
- ["java.nio.file", "Files", False, "copy", "(InputStream,Path,CopyOption[])", "", "Argument[1]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "copy", "", "", "Argument[1]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "createDirectories", "", "", "Argument[0]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "createDirectory", "", "", "Argument[0]", "path-injection", "manual"]
- ["java.nio.file", "Files", False, "createFile", "", "", "Argument[0]", "path-injection", "manual"]
@@ -40,6 +38,13 @@ extensions:
- ["java.nio.file", "Files", True, "delete", "(Path)", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.nio.file", "Files", True, "newInputStream", "(Path,OpenOption[])", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.nio.file", "Files", True, "newOutputStream", "(Path,OpenOption[])", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.nio.file", "FileSystem", False, "getPath", "", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation
- ["java.nio.file", "Path", False, "of", "(String,String[])", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation
- ["java.nio.file", "Path", False, "of", "(URI)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation
- ["java.nio.file", "Path", False, "resolve", "(String)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation
- ["java.nio.file", "Path", False, "resolveSibling", "(String)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation
- ["java.nio.file", "Paths", False, "get", "(String,String[])", "", "Argument[0..1]", "path-injection", "manual"] # old PathCreation
- ["java.nio.file", "Paths", False, "get", "(URI)", "", "Argument[0]", "path-injection", "manual"] # old PathCreation
- ["java.nio.file", "SecureDirectoryStream", True, "deleteDirectory", "(Path)", "", "Argument[0]", "path-injection", "ai-manual"]
- ["java.nio.file", "SecureDirectoryStream", True, "deleteFile", "(Path)", "", "Argument[0]", "path-injection", "ai-manual"]
- addsTo:

1
java/ql/lib/ext/java.nio.model.yml
View File

@@ -6,6 +6,7 @@ extensions:
- ["java.nio", "ByteBuffer", False, "array", "()", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- ["java.nio", "ByteBuffer", False, "get", "", "", "Argument[this]", "ReturnValue", "taint", "manual"]
- ["java.nio", "ByteBuffer", False, "wrap", "(byte[])", "", "Argument[0]", "ReturnValue", "taint", "manual"]
- ["java.nio", "Paths", False, "get", "(URI)", "", "Argument[0]", "ReturnValue", "taint", "manual"] # old PathCreation
- addsTo:
pack: codeql/java-all

13
java/ql/lib/semmle/code/java/security/TaintedPathQuery.qll
View File

@@ -5,7 +5,6 @@ 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.PathCreation
import semmle.code.java.security.PathSanitizer
/**
@@ -55,11 +54,7 @@ private class TaintPreservingUriCtorParam extends Parameter {
module TaintedPathConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
predicate isSink(DataFlow::Node sink) {
sink.asExpr() = any(PathCreation p).getAnInput()
or
sinkNode(sink, "path-injection")
}
predicate isSink(DataFlow::Node sink) { sinkNode(sink, "path-injection") }
predicate isBarrier(DataFlow::Node sanitizer) {
sanitizer.getType() instanceof BoxedType or
@@ -82,11 +77,7 @@ module TaintedPathFlow = TaintTracking::Global<TaintedPathConfig>;
module TaintedPathLocalConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof LocalUserInput }
predicate isSink(DataFlow::Node sink) {
sink.asExpr() = any(PathCreation p).getAnInput()
or
sinkNode(sink, "path-injection")
}
predicate isSink(DataFlow::Node sink) { sinkNode(sink, "path-injection") }
predicate isBarrier(DataFlow::Node sanitizer) {
sanitizer.getType() instanceof BoxedType or

1
java/ql/src/Security/CWE/CWE-022/TaintedPath.ql
View File

@@ -14,6 +14,7 @@
*/
import java
import semmle.code.java.security.PathCreation
import semmle.code.java.security.TaintedPathQuery
import TaintedPathFlow::PathGraph

1
java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql
View File

@@ -14,6 +14,7 @@
*/
import java
import semmle.code.java.security.PathCreation
import semmle.code.java.security.TaintedPathQuery
import TaintedPathLocalFlow::PathGraph

3
java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql
View File

@@ -16,7 +16,6 @@ import java
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.dataflow.ExternalFlow
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.security.PathCreation
import JFinalController
import semmle.code.java.security.PathSanitizer
import InjectFilePathFlow::PathGraph
@@ -52,7 +51,7 @@ module InjectFilePathConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
predicate isSink(DataFlow::Node sink) {
sink.asExpr() = any(PathCreation p).getAnInput() and
sinkNode(sink, "path-injection") and
not sink instanceof NormalizedPathNode
}

11
java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected
View File

@@ -2,7 +2,12 @@ edges
| FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath |
| FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath |
| FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath |
| FilePathInjection.java:177:50:177:58 | file : File | FilePathInjection.java:182:30:182:33 | file |
| FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath |
| FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath : String |
| FilePathInjection.java:209:15:209:32 | new File(...) : File | FilePathInjection.java:217:19:217:22 | file : File |
| FilePathInjection.java:209:24:209:31 | filePath : String | FilePathInjection.java:209:15:209:32 | new File(...) : File |
| FilePathInjection.java:217:19:217:22 | file : File | FilePathInjection.java:177:50:177:58 | file : File |
nodes
| FilePathInjection.java:21:21:21:34 | getPara(...) : String | semmle.label | getPara(...) : String |
| FilePathInjection.java:26:47:26:59 | finalFilePath | semmle.label | finalFilePath |
@@ -10,11 +15,17 @@ nodes
| FilePathInjection.java:72:47:72:59 | finalFilePath | semmle.label | finalFilePath |
| FilePathInjection.java:87:21:87:34 | getPara(...) : String | semmle.label | getPara(...) : String |
| FilePathInjection.java:95:47:95:59 | finalFilePath | semmle.label | finalFilePath |
| FilePathInjection.java:177:50:177:58 | file : File | semmle.label | file : File |
| FilePathInjection.java:182:30:182:33 | file | semmle.label | file |
| FilePathInjection.java:205:17:205:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
| FilePathInjection.java:209:15:209:32 | new File(...) : File | semmle.label | new File(...) : File |
| FilePathInjection.java:209:24:209:31 | filePath | semmle.label | filePath |
| FilePathInjection.java:209:24:209:31 | filePath : String | semmle.label | filePath : String |
| FilePathInjection.java:217:19:217:22 | file : File | semmle.label | file : File |
subpaths
#select
| FilePathInjection.java:26:47:26:59 | finalFilePath | FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:21:21:21:34 | getPara(...) | user-provided value |
| FilePathInjection.java:72:47:72:59 | finalFilePath | FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:72:47:72:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:64:21:64:34 | getPara(...) | user-provided value |
| FilePathInjection.java:95:47:95:59 | finalFilePath | FilePathInjection.java:87:21:87:34 | getPara(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:87:21:87:34 | getPara(...) | user-provided value |
| FilePathInjection.java:182:30:182:33 | file | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:182:30:182:33 | file | External control of file name or path due to $@. | FilePathInjection.java:205:17:205:44 | getParameter(...) | user-provided value |
| FilePathInjection.java:209:24:209:31 | filePath | FilePathInjection.java:205:17:205:44 | getParameter(...) : String | FilePathInjection.java:209:24:209:31 | filePath | External control of file name or path due to $@. | FilePathInjection.java:205:17:205:44 | getParameter(...) | user-provided value |

1
java/ql/test/query-tests/Telemetry/SupportedExternalSinks/SupportedExternalSinks.expected
View File

@@ -1,2 +1,3 @@
| java.io.File#File(String) | 1 |
| java.io.FileWriter#FileWriter(File) | 1 |
| java.net.URL#openStream() | 1 |