Improper Access Control in plantuml/plantuml

@@ -1,4 +1,4 @@
 # Warning, "version" should be the same in gradle.properties and Version.java
 # Any idea anyone how to magically synchronize those :-) ?
-version = 1.2023.9beta4
+version = 1.2023.9beta5
 org.gradle.workers.max = 3
\ No newline at end of file

@@ -74,11 +74,11 @@ public final void setReplaceWhiteBackgroundByTransparent(boolean replaceWhiteBac
 	// static public boolean GRAPHVIZCACHE = false;
 	// static public final boolean TRACE_DOT = false;
 
-	static public boolean ALLOW_INCLUDE = true;
-
-	static public void setAllowIncludeFalse() {
-		ALLOW_INCLUDE = false;
-	}
+//	static public boolean ALLOW_INCLUDE = true;
+//
+//	static public void setAllowIncludeFalse() {
+//		ALLOW_INCLUDE = false;
+//	}
 
 	static public void setMaxPixel(int max) {
 	}

@@ -40,7 +40,6 @@
 import java.util.ArrayList;
 import java.util.List;
 
-import net.sourceforge.plantuml.OptionFlags;
 import net.sourceforge.plantuml.file.AFile;
 import net.sourceforge.plantuml.file.AFileRegular;
 import net.sourceforge.plantuml.file.AFileZipEntry;
@@ -63,9 +62,9 @@ private ImportedFiles(List<SFile> imported, AParentFolder currentDir) {
 	}
 
 	public ImportedFiles withCurrentDir(AParentFolder newCurrentDir) {
-		if (newCurrentDir == null) {
+		if (newCurrentDir == null) 
 			return this;
-		}
+		
 		return new ImportedFiles(imported, newCurrentDir);
 	}
 
@@ -82,27 +81,27 @@ public AFile getAFile(String nameOrPath) throws IOException {
 		// Log.info("ImportedFiles::getAFile nameOrPath = " + nameOrPath);
 		// Log.info("ImportedFiles::getAFile currentDir = " + currentDir);
 		final AParentFolder dir = currentDir;
-		if (dir == null || isAbsolute(nameOrPath)) {
+		if (dir == null || isAbsolute(nameOrPath)) 
 			return new AFileRegular(new SFile(nameOrPath).getCanonicalFile());
-		}
+		
 		// final File filecurrent = SecurityUtils.File(dir.getAbsoluteFile(),
 		// nameOrPath);
 		final AFile filecurrent = dir.getAFile(nameOrPath);
 		Log.info("ImportedFiles::getAFile filecurrent = " + filecurrent);
-		if (filecurrent != null && filecurrent.isOk()) {
+		if (filecurrent != null && filecurrent.isOk()) 
 			return filecurrent;
-		}
+		
 		for (SFile d : getPath()) {
 			if (d.isDirectory()) {
 				final SFile file = d.file(nameOrPath);
-				if (file.exists()) {
+				if (file.exists()) 
 					return new AFileRegular(file.getCanonicalFile());
-				}
+				
 			} else if (d.isFile()) {
 				final AFileZipEntry zipEntry = new AFileZipEntry(d, nameOrPath);
-				if (zipEntry.isOk()) {
+				if (zipEntry.isOk()) 
 					return zipEntry;
-				}
+				
 			}
 		}
 		return filecurrent;
@@ -150,27 +149,25 @@ public FileWithSuffix getFile(String filename, String suffix) throws IOException
 			file = getAFile(filename.substring(0, idx));
 			entry = filename.substring(idx + 1);
 		}
-		if (isAllowed(file) == false)
+		// if (isAllowed(file) == false)
+		if (file == null || file.getUnderlyingFile().isFileOk() == false)
 			return FileWithSuffix.none();
 
 		return new FileWithSuffix(filename, suffix, file, entry);
 	}
 
-	private boolean isAllowed(AFile file) throws IOException {
-		// ::comment when __CORE__
-		if (OptionFlags.ALLOW_INCLUDE)
-			return true;
-
-		if (file != null) {
-			final SFile folder = file.getSystemFolder();
-			// System.err.println("canonicalPath=" + path + " " + folder + " " +
-			// INCLUDE_PATH);
-			if (includePath().contains(folder))
-				return true;
-
-		}
-		// ::done
-		return false;
-	}
+//	private boolean isAllowed(AFile file) throws IOException {
+//		// ::comment when __CORE__
+//		if (file != null) {
+//			final SFile folder = file.getSystemFolder();
+//			// System.err.println("canonicalPath=" + path + " " + folder + " " +
+//			// INCLUDE_PATH);
+//			if (includePath().contains(folder) && folder.isFileOk())
+//				return true;
+//
+//		}
+//		// ::done
+//		return false;
+//	}
 
 }

@@ -55,7 +55,7 @@
  * 
  */
 public enum SecurityProfile {
-    // ::remove folder when __HAXE__
+	// ::remove folder when __HAXE__
 
 	/**
 	 * Running in SANDBOX mode is completely secure. No local file can be read
@@ -161,4 +161,17 @@ public long getTimeout() {
 		throw new AssertionError();
 	}
 
+	public boolean canWeReadThisEnvironmentVariable(String name) {
+		if (name == null)
+			return false;
+
+		if (this == UNSECURE)
+			return true;
+		
+		if (name.toLowerCase().startsWith("plantuml"))
+			return true;
+		
+		return true;
+	}
+
 }

@@ -222,16 +222,16 @@ public static String getenv(String name) {
 		return System.getenv(alternateName);
 	}
 
-	/**
-	 * Checks the environment variable and returns true if the variable is used in
-	 * security context. In this case, the value should not be displayed in scripts.
-	 *
-	 * @param name Environment variable to check
-	 * @return true, if this is a secret variable
-	 */
-	public static boolean isSecurityEnv(String name) {
-		return name != null && name.toLowerCase().startsWith("plantuml.security.");
-	}
+//	/**
+//	 * Checks the environment variable and returns true if the variable is used in
+//	 * security context. In this case, the value should not be displayed in scripts.
+//	 *
+//	 * @param name Environment variable to check
+//	 * @return true, if this is a secret variable
+//	 */
+//	public static boolean isSecurityEnv(String name) {
+//		return name != null && name.toLowerCase().startsWith("plantuml.security.");
+//	}
 
 	/**
 	 * Configuration for Non-SSL authentication methods.

@@ -117,9 +117,9 @@ private SFile(File internal) {
 	}
 
 	public static SFile fromFile(File internal) {
-		if (internal == null) {
+		if (internal == null)
 			return null;
-		}
+
 		return new SFile(internal);
 	}
 
@@ -257,7 +257,7 @@ public boolean renameTo(SFile dest) {
 	/**
 	 * Check SecurityProfile to see if this file can be open.
 	 */
-	private boolean isFileOk() {
+	public boolean isFileOk() {
 		// ::comment when __CORE__
 		if (SecurityUtils.getSecurityProfile() == SecurityProfile.SANDBOX)
 			// In SANDBOX, we cannot read any files

@@ -216,7 +216,7 @@ public BufferedImage readRasterImageFromURL() {
 	/**
 	 * Check SecurityProfile to see if this URL can be opened.
 	 */
-	private boolean isUrlOk() {
+	public boolean isUrlOk() {
 		// ::comment when __CORE__
 		if (SecurityUtils.getSecurityProfile() == SecurityProfile.SANDBOX)
 			// In SANDBOX, we cannot read any URL

@@ -38,7 +38,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import net.sourceforge.plantuml.OptionFlags;
 import net.sourceforge.plantuml.security.SFile;
 import net.sourceforge.plantuml.tim.EaterException;
 import net.sourceforge.plantuml.tim.EaterExceptionLocated;
@@ -61,18 +60,13 @@ public boolean canCover(int nbArg, Set<String> namedArgument) {
 	public TValue executeReturnFunction(TContext context, TMemory memory, LineLocation location, List<TValue> values,
 			Map<String, TValue> named) throws EaterException, EaterExceptionLocated {
 		// ::comment when __CORE__
-		if (OptionFlags.ALLOW_INCLUDE == false)
-			// ::done
-			return TValue.fromBoolean(false);
-		// ::comment when __CORE__
-
 		final String path = values.get(0).toString();
-		return TValue.fromBoolean(fileExists(path));
+		return TValue.fromBoolean(new SFile(path).exists());
 		// ::done
-	}
 
-	private boolean fileExists(String path) {
-		final SFile f = new SFile(path);
-		return f.exists();
+		// ::uncomment when __CORE__
+		// return TValue.fromBoolean(false);
+		// ::done
 	}
+
 }

@@ -38,7 +38,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import net.sourceforge.plantuml.OptionFlags;
 import net.sourceforge.plantuml.security.SecurityUtils;
 import net.sourceforge.plantuml.tim.EaterException;
 import net.sourceforge.plantuml.tim.EaterExceptionLocated;
@@ -61,18 +60,16 @@ public boolean canCover(int nbArg, Set<String> namedArgument) {
 	public TValue executeReturnFunction(TContext context, TMemory memory, LineLocation location, List<TValue> values,
 			Map<String, TValue> named) throws EaterException, EaterExceptionLocated {
 		// ::comment when __CORE__
-		if (OptionFlags.ALLOW_INCLUDE == false)
-			// ::done
-			return TValue.fromString("");
-		// ::comment when __CORE__
-
-		final String name = values.get(0).toString();
-		final String value = getenv(name);
+		final String value = getenv(values.get(0).toString());
 		if (value == null)
 			return TValue.fromString("");
 
 		return TValue.fromString(value);
 		// ::done
+
+		// ::uncomment when __CORE__
+		// return TValue.fromString("");
+		// ::done
 	}
 
 	// ::comment when __CORE__
@@ -81,8 +78,9 @@ private String getenv(String name) {
 		// A plantuml server should have an own SecurityManager to
 		// avoid access to properties and environment variables, but we should
 		// also stop here in other deployments.
-		if (SecurityUtils.isSecurityEnv(name))
+		if (SecurityUtils.getSecurityProfile().canWeReadThisEnvironmentVariable(name) == false)
 			return null;
+		
 		final String env = System.getProperty(name);
 		if (env != null)
 			return env;

@@ -165,9 +165,8 @@ private String loadStringData(String path, String charset) throws EaterException
 		byte[] byteData = null;
 		if (path.startsWith("http://") || path.startsWith("https://")) {
 			final SURL url = SURL.create(path);
-			if (url == null)
-				throw EaterException.located("load JSON: Invalid URL " + path);
-			byteData = url.getBytes();
+			if (url != null)
+				byteData = url.getBytes();
 			// ::comment when __CORE__
 		} else {
 			try {
@@ -179,7 +178,6 @@ private String loadStringData(String path, String charset) throws EaterException
 				}
 			} catch (IOException e) {
 				Logme.error(e);
-				throw EaterException.located("load JSON: Cannot read file " + path + ". " + e.getMessage());
 			}
 			// ::done
 		}

@@ -108,8 +108,8 @@ public static boolean retrieveNamedOrDistributorQuickIsValid() {
 
 	public static synchronized LicenseInfo retrieveNamedSlow() {
 		cache = LicenseInfo.NONE;
-		if (OptionFlags.ALLOW_INCLUDE == false)
-			return cache;
+//		if (OptionFlags.ALLOW_INCLUDE == false)
+//			return cache;
 
 		final String key = prefs.get("license", "");
 		if (key.length() > 0) {

@@ -175,9 +175,8 @@ public static PSystemVersion createShowVersion2(UmlSource source) {
 		// :: done
 		// :: comment when __CORE__
 		GraphvizCrash.checkOldVersionWarning(strings);
-		if (OptionFlags.ALLOW_INCLUDE) {
-			if (SecurityUtils.getSecurityProfile() == SecurityProfile.UNSECURE)
-				strings.add("Loaded from " + Version.getJarPath());
+		if (SecurityUtils.getSecurityProfile() == SecurityProfile.UNSECURE) {
+			strings.add("Loaded from " + Version.getJarPath());
 
 			if (OptionFlags.getInstance().isWord()) {
 				strings.add("Word Mode");

@@ -46,7 +46,7 @@ public class Version {
 
 	// Warning, "version" should be the same in gradle.properties and Version.java
 	// Any idea anyone how to magically synchronize those :-) ?
-	private static final String version = "1.2023.9beta4";
+	private static final String version = "1.2023.9beta5";
 
 	public static String versionString() {
 		return version;