Skip to content

Commit ce03aeb

Browse files
luchua-bcluchua-bc
committed
Fixed an issue related to normalized path
1 parent 4609227 commit ce03aeb

File tree

4 files changed

+30
-9
lines changed

4 files changed

+30
-9
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ class InjectFilePathConfig extends TaintTracking::Configuration {
2424
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
2525

2626
override predicate isSink(DataFlow::Node sink) {
27-
sink.asExpr() = any(PathCreation p).getAnInput()
27+
sink.asExpr() = any(PathCreation p).getAnInput() and
28+
not sink instanceof SanitizedNode
2829
}
2930

3031
override predicate isSanitizer(DataFlow::Node node) {

‎java/ql/src/experimental/Security/CWE/CWE-073/PathSanitizer.qll

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,25 @@ private class UrlDecodeSanitizer extends MethodAccess {
173173
this.getMethod().hasName("decode")
174174
}
175175
}
176+
177+
/** A sanitized node that is protected against path traversal vulnerabilities. */
178+
abstract class SanitizedNode extends DataFlow::Node { }
179+
180+
class NodeWithPathNormalizer extends SanitizedNode {
181+
NodeWithPathNormalizer() {
182+
exists(MethodAccess ma |
183+
DataFlow::localExprFlow(this.asExpr(), ma) and ma instanceof PathNormalizeSanitizer
184+
)
185+
}
186+
}
187+
188+
/** Data model related to `java.nio.file.Path`. */
189+
private class PathDataModel extends SummaryModelCsv {
190+
override predicate row(string row) {
191+
row =
192+
[
193+
"java.nio.file;Paths;true;get;;;Argument[0];ReturnValue;value",
194+
"java.nio.file;Path;true;normalize;;;Argument[-1];ReturnValue;value"
195+
]
196+
}
197+
}

‎java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.expected

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
edges
22
| FilePathInjection.java:21:21:21:34 | getPara(...) : String | FilePathInjection.java:26:47:26:59 | finalFilePath |
3-
| FilePathInjection.java:41:21:41:34 | getPara(...) : String | FilePathInjection.java:44:25:44:37 | finalFilePath |
43
| FilePathInjection.java:64:21:64:34 | getPara(...) : String | FilePathInjection.java:65:31:65:38 | savePath : String |
54
| FilePathInjection.java:65:3:65:39 | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String | FilePathInjection.java:66:29:66:55 | this <.method> [com.jfinal.core.Controller.session, <map.value>] : String |
65
| FilePathInjection.java:65:31:65:38 | savePath : String | FilePathInjection.java:65:3:65:39 | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String |
@@ -11,12 +10,10 @@ edges
1110
| FilePathInjection.java:88:24:88:31 | savePath : String | FilePathInjection.java:88:3:88:32 | this <.method> [post update] [com.jfinal.core.Controller.request, <map.value>] : String |
1211
| FilePathInjection.java:89:29:89:48 | getAttr(...) : String | FilePathInjection.java:95:47:95:59 | finalFilePath |
1312
| FilePathInjection.java:89:29:89:48 | this <.method> [com.jfinal.core.Controller.request, <map.value>] : String | FilePathInjection.java:89:29:89:48 | getAttr(...) : String |
14-
| FilePathInjection.java:158:17:158:44 | getParameter(...) : String | FilePathInjection.java:162:24:162:31 | filePath |
13+
| FilePathInjection.java:159:17:159:44 | getParameter(...) : String | FilePathInjection.java:163:24:163:31 | filePath |
1514
nodes
1615
| FilePathInjection.java:21:21:21:34 | getPara(...) : String | semmle.label | getPara(...) : String |
1716
| FilePathInjection.java:26:47:26:59 | finalFilePath | semmle.label | finalFilePath |
18-
| FilePathInjection.java:41:21:41:34 | getPara(...) : String | semmle.label | getPara(...) : String |
19-
| FilePathInjection.java:44:25:44:37 | finalFilePath | semmle.label | finalFilePath |
2017
| FilePathInjection.java:64:21:64:34 | getPara(...) : String | semmle.label | getPara(...) : String |
2118
| FilePathInjection.java:65:3:65:39 | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String | semmle.label | this <.method> [post update] [com.jfinal.core.Controller.session, <map.value>] : String |
2219
| FilePathInjection.java:65:31:65:38 | savePath : String | semmle.label | savePath : String |
@@ -29,12 +26,11 @@ nodes
2926
| FilePathInjection.java:89:29:89:48 | getAttr(...) : String | semmle.label | getAttr(...) : String |
3027
| FilePathInjection.java:89:29:89:48 | this <.method> [com.jfinal.core.Controller.request, <map.value>] : String | semmle.label | this <.method> [com.jfinal.core.Controller.request, <map.value>] : String |
3128
| FilePathInjection.java:95:47:95:59 | finalFilePath | semmle.label | finalFilePath |
32-
| FilePathInjection.java:158:17:158:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
33-
| FilePathInjection.java:162:24:162:31 | filePath | semmle.label | filePath |
29+
| FilePathInjection.java:159:17:159:44 | getParameter(...) : String | semmle.label | getParameter(...) : String |
30+
| FilePathInjection.java:163:24:163:31 | filePath | semmle.label | filePath |
3431
subpaths
3532
#select
3633
| 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 |
37-
| FilePathInjection.java:44:25:44:37 | finalFilePath | FilePathInjection.java:41:21:41:34 | getPara(...) : String | FilePathInjection.java:44:25:44:37 | finalFilePath | External control of file name or path due to $@. | FilePathInjection.java:41:21:41:34 | getPara(...) | user-provided value |
3834
| 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 |
3935
| 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 |
40-
| FilePathInjection.java:162:24:162:31 | filePath | FilePathInjection.java:158:17:158:44 | getParameter(...) : String | FilePathInjection.java:162:24:162:31 | filePath | External control of file name or path due to $@. | FilePathInjection.java:158:17:158:44 | getParameter(...) | user-provided value |
36+
| FilePathInjection.java:163:24:163:31 | filePath | FilePathInjection.java:159:17:159:44 | getParameter(...) : String | FilePathInjection.java:163:24:163:31 | filePath | External control of file name or path due to $@. | FilePathInjection.java:159:17:159:44 | getParameter(...) | user-provided value |

‎java/ql/test/experimental/query-tests/security/CWE-073/FilePathInjection.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ private void readFile(HttpServletResponse resp, File file) {
153153
}
154154
}
155155

156+
// BAD: Download file to user specified path without validation
156157
public void downloadFile() throws FileNotFoundException, IOException {
157158
HttpServletRequest request = getRequest();
158159
String path = request.getParameter("path");
@@ -173,6 +174,7 @@ public void downloadFile() throws FileNotFoundException, IOException {
173174
}
174175
}
175176

177+
// GOOD: Download file with path validation
176178
public void downloadFile2() throws FileNotFoundException, IOException {
177179
HttpServletRequest request = getRequest();
178180
String path = request.getParameter("path");

0 commit comments

Comments
 (0)