Uncontrolled data used in path expression on RemoteXGBoostUploadServlet

[odaysec]

h2o-3/h2o-extensions/xgboost/src/main/java/hex/tree/xgboost/remote/RemoteXGBoostUploadServlet.java

Line 27 in e6a314b

if (uploadDir.mkdirs()) {

h2o-3/h2o-extensions/xgboost/src/main/java/hex/tree/xgboost/remote/RemoteXGBoostUploadServlet.java

Lines 22 to 30 in e6a314b

	return new File(H2O.ICE_ROOT.toString(), key);
	}
	public static File getCheckpointFile(String key) {
	File uploadDir = getUploadDir(key);
	if (uploadDir.mkdirs()) {
	LOG.debug("Created temporary directory " + uploadDir);
	}
	return new File(getUploadDir(key), "checkpoint.bin");

Accessing paths controlled by users can allow an attacker to access unexpected resources. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files. Paths that are naively constructed from data controlled by a user may be absolute paths, or may contain unexpected special characters such as "..". Such a path could point anywhere on the file system.

POC

In this a file name is read from a java.net.Socket and then used to access a file and send it back over the socket. However, a malicious user could enter a file name anywhere on the file system, such as "/etc/passwd" or "../../../etc/passwd".

public void sendUserFile(Socket sock, String user) {
	BufferedReader filenameReader = new BufferedReader(
			new InputStreamReader(sock.getInputStream(), "UTF-8"));
	String filename = filenameReader.readLine();
	// BAD: read from a file without checking its path
	BufferedReader fileReader = new BufferedReader(new FileReader(filename));
	String fileLine = fileReader.readLine();
	while(fileLine != null) {
		sock.getOutputStream().write(fileLine.getBytes());
		fileLine = fileReader.readLine();
	}
}

If the input should only be a file name, you can check that it doesn't contain any path separators or ".." sequences.

public void sendUserFileGood(Socket sock, String user) {
	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();
	}	
}

If the input should be within a specific directory, you can check that the resolved path is still contained within that directory.

public void sendUserFileGood(Socket sock, String user) {
	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();

	// 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();
	}
}

References

Path Traversal
CWE-22
CWE-23
CWE-36
CWE-73