Simplify test case and minor update to the query

Author: luchua-bc

java/ql/src/experimental/Security/CWE/CWE-552/UnsafeUrlForward.qll

@@ -200,7 +200,7 @@ private class LoadSpringResourceFlowStep extends SummaryModelCsv {
       [
         "org.springframework.core.io;ClassPathResource;false;ClassPathResource;;;Argument[0];Argument[-1];taint",
         "org.springframework.core.io;ClassPathResource;true;" +
-          ["getFilename", "getPath", "getURL", "resolveURL"] + ";;;Argument[-1];ReturnValue;value",
+          ["getFilename", "getPath", "getURL", "resolveURL"] + ";;;Argument[-1];ReturnValue;taint",
         "org.springframework.core.io;ResourceLoader;true;getResource;;;Argument[0];ReturnValue;taint",
         "org.springframework.core.io;Resource;true;createRelative;;;Argument[0];ReturnValue;taint"
       ]

java/ql/test/experimental/query-tests/security/CWE-552/UnsafeLoadSpringResource.java

@@ -45,7 +45,7 @@ public String getFileContent1(@RequestParam(name="fileName") String fileName) {
 	//GOOD: Get resource from ClassPathResource with input path validation
 	public String getFileContent1a(@RequestParam(name="fileName") String fileName) {
 		String result = null;
-		if (!isInvalidPath(fileName) && !isInvalidEncodedPath(fileName)) {
+		if (fileName.startsWith("/safe_dir") && !fileName.contains("..")) {
 			ClassPathResource clr = new ClassPathResource(fileName);
 			char[] buffer = new char[4096];
 			StringBuilder out = new StringBuilder();
@@ -86,9 +86,9 @@ public String getFileContent2(@RequestParam(name="fileName") String fileName) {
 	public String getFileContent2a(@RequestParam(name="fileName") String fileName) {
 		String content = null;
 
-		if (!isInvalidPath(fileName) && !isInvalidEncodedPath(fileName)) {
+		if (fileName.startsWith("/safe_dir") && !fileName.contains("..")) {
 			try {
-				File file = ResourceUtils.getFile(fileName);
+					File file = ResourceUtils.getFile(fileName);
 				//Read File Content
 				content = new String(Files.readAllBytes(file.toPath()));
 			} catch (IOException ie) {
@@ -132,7 +132,7 @@ public String getFileContent3(@RequestParam(name="fileName") String fileName) {
 	public String getFileContent3a(@RequestParam(name="fileName") String fileName) {
 		String content = null;
 
-		if (!isInvalidPath(fileName) && !isInvalidEncodedPath(fileName)) {
+		if (fileName.startsWith("/safe_dir") && !fileName.contains("..")) {
 			try {
 				Resource resource = resourceLoader.getResource(fileName);
 
@@ -150,121 +150,4 @@ public String getFileContent3a(@RequestParam(name="fileName") String fileName) {
 		}
 		return content;
 	}
-
-	/**
-	 * Check whether the given path contains invalid escape sequences.
-	 * @param path the path to validate
-	 * @return {@code true} if the path is invalid, {@code false} otherwise
-	 * @see Referenced code for path validation fix in https://github.com/mpgn/CVE-2019-3799
-	 */
-	private boolean isInvalidEncodedPath(String path) {
-		if (path.contains("%")) {
-			try {
-				// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
-				String decodedPath = URLDecoder.decode(path, "UTF-8");
-				if (isInvalidPath(decodedPath)) {
-					return true;
-				}
-				decodedPath = processPath(decodedPath);
-				if (isInvalidPath(decodedPath)) {
-					return true;
-				}
-			} catch (IllegalArgumentException | UnsupportedEncodingException ex) {
-				// Should never happen...
-			}
-		}
-		return false;
-	}
-
-	/**
-	 * Process the given resource path.
-	 * <p>The default implementation replaces:
-	 * <ul>
-	 * <li>Backslash with forward slash.
-	 * <li>Duplicate occurrences of slash with a single slash.
-	 * <li>Any combination of leading slash and control characters (00-1F and 7F)
-	 * with a single "/" or "". For example {@code "  / // foo/bar"}
-	 * becomes {@code "/foo/bar"}.
-	 * </ul>
-	 * @since 3.2.12
-	 * @see Referenced code for path validation fix in https://github.com/mpgn/CVE-2019-3799
-	 */
-	protected String processPath(String path) {
-		path = StringUtils.replace(path, "\\", "/");
-		path = cleanDuplicateSlashes(path);
-		return cleanLeadingSlash(path);
-	}
-
-	/** @see Referenced code for path validation fix in https://github.com/mpgn/CVE-2019-3799 **/
-	private String cleanDuplicateSlashes(String path) {
-		StringBuilder sb = null;
-		char prev = 0;
-		for (int i = 0; i < path.length(); i++) {
-			char curr = path.charAt(i);
-			try {
-				if ((curr == '/') && (prev == '/')) {
-					if (sb == null) {
-						sb = new StringBuilder(path.substring(0, i));
-					}
-					continue;
-				}
-				if (sb != null) {
-					sb.append(path.charAt(i));
-				}
-			} finally {
-				prev = curr;
-			}
-		}
-		return sb != null ? sb.toString() : path;
-	}
-
-	/** @see Referenced code for path validation fix in https://github.com/mpgn/CVE-2019-3799 **/
-	private String cleanLeadingSlash(String path) {
-		boolean slash = false;
-		for (int i = 0; i < path.length(); i++) {
-			if (path.charAt(i) == '/') {
-				slash = true;
-			}
-			else if (path.charAt(i) > ' ' && path.charAt(i) != 127) {
-				if (i == 0 || (i == 1 && slash)) {
-					return path;
-				}
-				return (slash ? "/" + path.substring(i) : path.substring(i));
-			}
-		}
-		return (slash ? "/" : "");
-	}
-
-	/**
-	 * Identifies invalid resource paths. By default rejects:
-	 * <ul>
-	 * <li>Paths that contain "WEB-INF" or "META-INF"
-	 * <li>Paths that contain "../" after a call to
-	 * {@link org.springframework.util.StringUtils#cleanPath}.
-	 * <li>Paths that represent a {@link org.springframework.util.ResourceUtils#isUrl
-	 * valid URL} or would represent one after the leading slash is removed.
-	 * </ul>
-	 * <p><strong>Note:</strong> this method assumes that leading, duplicate '/'
-	 * or control characters (e.g. white space) have been trimmed so that the
-	 * path starts predictably with a single '/' or does not have one.
-	 * @param path the path to validate
-	 * @return {@code true} if the path is invalid, {@code false} otherwise
-	 * @since 3.0.6
-	 * @see Referenced code for path validation fix in https://github.com/mpgn/CVE-2019-3799
-	 */
-	protected boolean isInvalidPath(String path) {
-		if (path.contains("WEB-INF") || path.contains("META-INF")) {
-			return true;
-		}
-		if (path.contains(":/")) {
-			String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path);
-			if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) {
-				return true;
-			}
-		}
-		if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) {
-			return true;
-		}
-		return false;
-	}
 }