Better path check for security files (SFile)

This commit is contained in:
Aljoscha Rittner 2021-11-22 15:41:49 +01:00
parent e195987b78
commit 31b4d0f29d
3 changed files with 27 additions and 8 deletions

View File

@ -52,6 +52,8 @@ import java.io.PrintWriter;
import java.io.UnsupportedEncodingException;
import java.net.URI;
import java.nio.charset.Charset;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@ -246,7 +248,11 @@ public class SFile implements Comparable<SFile> {
return false;
}
// In any case SFile should not access the security folders (the files must be handled internally)
if (isDenied()) {
try {
if (isDenied()) {
return false;
}
} catch (IOException e) {
return false;
}
// Files in "plantuml.include.path" and "plantuml.allowlist.path" are ok.
@ -291,13 +297,29 @@ public class SFile implements Comparable<SFile> {
* Checks, if the SFile is inside the folder (-structure) of the security area.
*
* @return true, if the file is not allowed to read/write
* @throws IOException If an I/O error occurs, which is possible because the check the pathname may require
* filesystem queries
*/
private boolean isDenied() {
private boolean isDenied() throws IOException {
SFile securityPath = SecurityUtils.getSecurityPath();
if (securityPath == null) {
return false;
}
return getCleanPathSecure().startsWith(securityPath.getCleanPathSecure());
return getSanitizedPath().startsWith(securityPath.getSanitizedPath());
}
/**
* Returns a sanitized, canonical and normalized Path to a file.
*
* @return the Path
* @throws IOException If an I/O error occurs, which is possible because the construction of the canonical pathname
* may require filesystem queries
* @see #getCleanPathSecure()
* @see File#getCanonicalPath()
* @see Path#normalize()
*/
private Path getSanitizedPath() throws IOException {
return Paths.get(new File(getCleanPathSecure()).getCanonicalPath()).normalize();
}
private String getCleanPathSecure() {

View File

@ -358,7 +358,6 @@ public class SecurityUtils {
File userCredentials = new File(securityFilePath, userToken + ".credential");
JsonValue jsonValue = loadJson(userCredentials);
return SecurityCredentials.fromJson(jsonValue);
}
}
return SecurityCredentials.NONE;

View File

@ -47,12 +47,10 @@ class SFileTest {
// A file is needed:
File secretFile = File.createTempFile("user", ".credentials", secureFolder);
assertThat(secretFile).describedAs("File should be visible with standard java.io.File")
.exists();
assertThat(secretFile).describedAs("File should be visible with standard java.io.File").exists();
SFile file = new SFile(secretFile.getAbsolutePath());
assertThat(file.exists()).describedAs("File should be invisible for SFile")
.isFalse();
assertThat(file.exists()).describedAs("File should be invisible for SFile").isFalse();
}
}