diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected index ff2a1aee759c..0e2d90c3709c 100644 --- a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected +++ b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.expected @@ -1,9 +1,14 @@ edges -| TaintedPath.java:11:38:11:110 | new BufferedReader(...) : BufferedReader | TaintedPath.java:12:24:12:37 | filenameReader : BufferedReader | -| TaintedPath.java:11:57:11:109 | new InputStreamReader(...) : InputStreamReader | TaintedPath.java:11:38:11:110 | new BufferedReader(...) : BufferedReader | -| TaintedPath.java:11:79:11:99 | getInputStream(...) : InputStream | TaintedPath.java:11:57:11:109 | new InputStreamReader(...) : InputStreamReader | -| TaintedPath.java:12:24:12:37 | filenameReader : BufferedReader | TaintedPath.java:12:24:12:48 | readLine(...) : String | -| TaintedPath.java:12:24:12:48 | readLine(...) : String | TaintedPath.java:14:68:14:75 | filename | +| TaintedPath.java:12:38:12:110 | new BufferedReader(...) : BufferedReader | TaintedPath.java:13:24:13:37 | filenameReader : BufferedReader | +| TaintedPath.java:12:57:12:109 | new InputStreamReader(...) : InputStreamReader | TaintedPath.java:12:38:12:110 | new BufferedReader(...) : BufferedReader | +| TaintedPath.java:12:79:12:99 | getInputStream(...) : InputStream | TaintedPath.java:12:57:12:109 | new InputStreamReader(...) : InputStreamReader | +| TaintedPath.java:13:24:13:37 | filenameReader : BufferedReader | TaintedPath.java:13:24:13:48 | readLine(...) : String | +| TaintedPath.java:13:24:13:48 | readLine(...) : String | TaintedPath.java:15:68:15:75 | filename | +| TaintedPath.java:38:41:39:70 | new BufferedReader(...) : BufferedReader | TaintedPath.java:40:27:40:40 | filenameReader : BufferedReader | +| TaintedPath.java:39:17:39:69 | new InputStreamReader(...) : InputStreamReader | TaintedPath.java:38:41:39:70 | new BufferedReader(...) : BufferedReader | +| TaintedPath.java:39:39:39:59 | getInputStream(...) : InputStream | TaintedPath.java:39:17:39:69 | new InputStreamReader(...) : InputStreamReader | +| TaintedPath.java:40:27:40:40 | filenameReader : BufferedReader | TaintedPath.java:40:27:40:51 | readLine(...) : String | +| TaintedPath.java:40:27:40:51 | readLine(...) : String | TaintedPath.java:43:46:43:53 | filename | | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:24:20:24:23 | temp | | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:27:21:27:24 | temp | | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:30:44:30:47 | temp | @@ -189,12 +194,18 @@ edges | mad/Test.java:221:26:221:33 | source(...) : String | mad/Test.java:221:19:221:33 | (...)... | | mad/Test.java:226:29:226:36 | source(...) : String | mad/Test.java:226:20:226:36 | (...)... | nodes -| TaintedPath.java:11:38:11:110 | new BufferedReader(...) : BufferedReader | semmle.label | new BufferedReader(...) : BufferedReader | -| TaintedPath.java:11:57:11:109 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader | -| TaintedPath.java:11:79:11:99 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| TaintedPath.java:12:24:12:37 | filenameReader : BufferedReader | semmle.label | filenameReader : BufferedReader | -| TaintedPath.java:12:24:12:48 | readLine(...) : String | semmle.label | readLine(...) : String | -| TaintedPath.java:14:68:14:75 | filename | semmle.label | filename | +| TaintedPath.java:12:38:12:110 | new BufferedReader(...) : BufferedReader | semmle.label | new BufferedReader(...) : BufferedReader | +| TaintedPath.java:12:57:12:109 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader | +| TaintedPath.java:12:79:12:99 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | +| TaintedPath.java:13:24:13:37 | filenameReader : BufferedReader | semmle.label | filenameReader : BufferedReader | +| TaintedPath.java:13:24:13:48 | readLine(...) : String | semmle.label | readLine(...) : String | +| TaintedPath.java:15:68:15:75 | filename | semmle.label | filename | +| TaintedPath.java:38:41:39:70 | new BufferedReader(...) : BufferedReader | semmle.label | new BufferedReader(...) : BufferedReader | +| TaintedPath.java:39:17:39:69 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader | +| TaintedPath.java:39:39:39:59 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | +| TaintedPath.java:40:27:40:40 | filenameReader : BufferedReader | semmle.label | filenameReader : BufferedReader | +| TaintedPath.java:40:27:40:51 | readLine(...) : String | semmle.label | readLine(...) : String | +| TaintedPath.java:43:46:43:53 | filename | semmle.label | filename | | Test.java:19:18:19:38 | getHostName(...) : String | semmle.label | getHostName(...) : String | | Test.java:24:20:24:23 | temp | semmle.label | temp | | Test.java:27:21:27:24 | temp | semmle.label | temp | @@ -386,7 +397,8 @@ nodes | mad/Test.java:226:29:226:36 | source(...) : String | semmle.label | source(...) : String | subpaths #select -| TaintedPath.java:14:53:14:76 | new FileReader(...) | TaintedPath.java:11:79:11:99 | getInputStream(...) : InputStream | TaintedPath.java:14:68:14:75 | filename | This path depends on a $@. | TaintedPath.java:11:79:11:99 | getInputStream(...) | user-provided value | +| TaintedPath.java:15:53:15:76 | new FileReader(...) | TaintedPath.java:12:79:12:99 | getInputStream(...) : InputStream | TaintedPath.java:15:68:15:75 | filename | This path depends on a $@. | TaintedPath.java:12:79:12:99 | getInputStream(...) | user-provided value | +| TaintedPath.java:43:25:43:54 | resolve(...) | TaintedPath.java:39:39:39:59 | getInputStream(...) : InputStream | TaintedPath.java:43:46:43:53 | filename | This path depends on a $@. | TaintedPath.java:39:39:39:59 | getInputStream(...) | user-provided value | | Test.java:24:11:24:24 | new File(...) | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:24:20:24:23 | temp | This path depends on a $@. | Test.java:19:18:19:38 | getHostName(...) | user-provided value | | Test.java:27:11:27:25 | get(...) | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:27:21:27:24 | temp | This path depends on a $@. | Test.java:19:18:19:38 | getHostName(...) | user-provided value | | Test.java:30:11:30:48 | getPath(...) | Test.java:19:18:19:38 | getHostName(...) : String | Test.java:30:44:30:47 | temp | This path depends on a $@. | Test.java:19:18:19:38 | getHostName(...) | user-provided value | diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java index 5a5a63ad6adc..a2ae561be588 100644 --- a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java +++ b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java @@ -1,10 +1,11 @@ import java.io.BufferedReader; +import java.io.File; import java.io.FileReader; +import java.io.IOException; import java.io.InputStreamReader; import java.net.Socket; import java.nio.file.Path; import java.nio.file.Paths; -import java.io.IOException; public class TaintedPath { public void sendUserFile(Socket sock, String user) throws IOException { @@ -32,4 +33,40 @@ public void sendUserFileGood(Socket sock, String user) throws IOException { } } } + + public void sendUserFileGood2(Socket sock, String user) throws Exception { + BufferedReader filenameReader = new BufferedReader( + new InputStreamReader(sock.getInputStream(), "UTF-8")); + String filename = filenameReader.readLine(); + + Path publicFolder = Paths.get("/home/" + user + "/public").normalize().toAbsolutePath(); + Path filePath = publicFolder.resolve(filename).normalize().toAbsolutePath(); // FP until the path-injection sinks are reworked + + // GOOD: ensure that the path stays within the public folder + if (!filePath.startsWith(publicFolder + File.separator)) { + throw new IllegalArgumentException("Invalid filename"); + } + BufferedReader fileReader = new BufferedReader(new FileReader(filePath.toString())); + String fileLine = fileReader.readLine(); + while(fileLine != null) { + sock.getOutputStream().write(fileLine.getBytes()); + fileLine = fileReader.readLine(); + } + } + + public void sendUserFileGood3(Socket sock, String user) throws Exception { + BufferedReader filenameReader = new BufferedReader( + new InputStreamReader(sock.getInputStream(), "UTF-8")); + String filename = filenameReader.readLine(); + // GOOD: ensure that the filename has no path separators or parent directory references + if (filename.contains("..") || filename.contains("/") || filename.contains("\\")) { + throw new IllegalArgumentException("Invalid filename"); + } + BufferedReader fileReader = new BufferedReader(new FileReader(filename)); + String fileLine = fileReader.readLine(); + while(fileLine != null) { + sock.getOutputStream().write(fileLine.getBytes()); + fileLine = fileReader.readLine(); + } + } }