hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-07-10 05:55:32 +02:00
Code Issues Packages Projects Releases Wiki Activity

Fix @Pattern sanitizer for TaintedPath

Browse Source
This commit is contained in:
Owen Mansel-Chan
2026-07-03 11:08:54 +01:00
parent 077b531e41
commit 76b4f4f223
3 changed files with 17 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
java/ql/lib/semmle/code/java/security/PathSanitizer.qll
View File

@@ -470,7 +470,12 @@ private class DirectoryCharactersGuard extends PathGuard {
Expr checkedExpr;
boolean branch;
DirectoryCharactersGuard() { isMatchesCall(this, checkedExpr, branch) }
DirectoryCharactersGuard() {
// Annotations are handled directly as barriers in `DirectoryCharactersSanitizer`,
// since they don't dominate the sanitized expression and so can't act as barrier guards.
not this instanceof Annotation and
isMatchesCall(this, checkedExpr, branch)
}
override Expr getCheckedExpr() { result = checkedExpr }
@@ -493,7 +498,15 @@ private predicate directoryCharactersGuard(Guard g, Expr e, boolean branch) {
*/
private class DirectoryCharactersSanitizer extends PathInjectionSanitizer {
DirectoryCharactersSanitizer() {
this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer or
this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer
or
this = DataFlow::BarrierGuard<directoryCharactersGuard/3>::getABarrierNode()
or
// Annotations don't fit into the model of barrier guards because the
// annotation doesn't dominate the sanitized expression, so we instead
// treat them as barriers directly.
exists(RegexMatch rm | rm instanceof Annotation and isMatchesCall(rm, _, true) |
this.asExpr() = rm.getString()
)
}
}

4
java/ql/test/query-tests/security/CWE-022/semmle/tests/SanitizationTests2.java
View File

@@ -21,8 +21,8 @@ public class SanitizationTests2 {
@GetMapping("/product/vuln/path-injection-fix")
public ResponseEntity<String> vulnerablePathInjectionFix(@RequestParam("path")
@javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9]*") String path) throws IOException { // $ SPURIOUS: Source
try (FileInputStream stream = new FileInputStream(path)) { // $ SPURIOUS: Alert
@javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9]*") String path) throws IOException {
try (FileInputStream stream = new FileInputStream(path)) {
byte[] fileContents = stream.readNBytes(1024);
return ResponseEntity.ok(new String(fileContents, StandardCharsets.UTF_8));
}

4
java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected
View File

@@ -1,6 +1,5 @@
#select
| SanitizationTests2.java:16:59:16:62 | path | SanitizationTests2.java:14:59:14:91 | path : String | SanitizationTests2.java:16:59:16:62 | path | This path depends on a $@. | SanitizationTests2.java:14:59:14:91 | path | user-provided value |
| SanitizationTests2.java:25:59:25:62 | path | SanitizationTests2.java:23:62:24:139 | path : String | SanitizationTests2.java:25:59:25:62 | path | This path depends on a $@. | SanitizationTests2.java:23:62:24:139 | path | user-provided value |
| TaintedPath.java:16:71:16:78 | filename | TaintedPath.java:13:58:13:78 | getInputStream(...) : InputStream | TaintedPath.java:16:71:16:78 | filename | This path depends on a $@. | TaintedPath.java:13:58:13:78 | getInputStream(...) | user-provided value |
| Test.java:37:52:37:68 | (...)... | Test.java:32:16:32:45 | getParameter(...) : String | Test.java:37:52:37:68 | (...)... | This path depends on a $@. | Test.java:32:16:32:45 | getParameter(...) | user-provided value |
| Test.java:39:32:39:48 | (...)... | Test.java:32:16:32:45 | getParameter(...) : String | Test.java:39:32:39:48 | (...)... | This path depends on a $@. | Test.java:32:16:32:45 | getParameter(...) | user-provided value |
@@ -80,7 +79,6 @@
| Test.java:204:20:204:36 | (...)... | Test.java:32:16:32:45 | getParameter(...) : String | Test.java:204:20:204:36 | (...)... | This path depends on a $@. | Test.java:32:16:32:45 | getParameter(...) | user-provided value |
edges
| SanitizationTests2.java:14:59:14:91 | path : String | SanitizationTests2.java:16:59:16:62 | path | provenance | Sink:MaD:23 |
| SanitizationTests2.java:23:62:24:139 | path : String | SanitizationTests2.java:25:59:25:62 | path | provenance | Sink:MaD:23 |
| TaintedPath.java:13:17:13:89 | new BufferedReader(...) : BufferedReader | TaintedPath.java:14:27:14:40 | filenameReader : BufferedReader | provenance | |
| TaintedPath.java:13:36:13:88 | new InputStreamReader(...) : InputStreamReader | TaintedPath.java:13:17:13:89 | new BufferedReader(...) : BufferedReader | provenance | MaD:74 |
| TaintedPath.java:13:58:13:78 | getInputStream(...) : InputStream | TaintedPath.java:13:36:13:88 | new InputStreamReader(...) : InputStreamReader | provenance | Src:MaD:72 MaD:76 |
@@ -318,8 +316,6 @@ models
nodes
| SanitizationTests2.java:14:59:14:91 | path : String | semmle.label | path : String |
| SanitizationTests2.java:16:59:16:62 | path | semmle.label | path |
| SanitizationTests2.java:23:62:24:139 | path : String | semmle.label | path : String |
| SanitizationTests2.java:25:59:25:62 | path | semmle.label | path |
| TaintedPath.java:13:17:13:89 | new BufferedReader(...) : BufferedReader | semmle.label | new BufferedReader(...) : BufferedReader |
| TaintedPath.java:13:36:13:88 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader |
| TaintedPath.java:13:58:13:78 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream |