diff --git a/src/net/sourceforge/plantuml/security/SFile.java b/src/net/sourceforge/plantuml/security/SFile.java index ed1193e90..8049bc11d 100644 --- a/src/net/sourceforge/plantuml/security/SFile.java +++ b/src/net/sourceforge/plantuml/security/SFile.java @@ -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 { 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 { * 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() { diff --git a/src/net/sourceforge/plantuml/security/SecurityUtils.java b/src/net/sourceforge/plantuml/security/SecurityUtils.java index 1c4962ca0..a67a5b9a1 100644 --- a/src/net/sourceforge/plantuml/security/SecurityUtils.java +++ b/src/net/sourceforge/plantuml/security/SecurityUtils.java @@ -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; diff --git a/test/net/sourceforge/plantuml/security/SFileTest.java b/test/net/sourceforge/plantuml/security/SFileTest.java index 1983095d9..b1fd4be67 100644 --- a/test/net/sourceforge/plantuml/security/SFileTest.java +++ b/test/net/sourceforge/plantuml/security/SFileTest.java @@ -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(); } }