keep the shared weaving cache out of the world-writable temp directory

weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCacheFactory.java

@@ -13,6 +13,17 @@
 package org.aspectj.weaver.tools.cache;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.file.FileAlreadyExistsException;
+import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.Set;
 
 import org.aspectj.weaver.Dump;
 
@@ -22,9 +33,13 @@ public class SimpleCacheFactory {
 	public static final String CACHE_DIR = "aj.weaving.cache.dir";
 	public static final String CACHE_IMPL = "aj.weaving.cache.impl";
 
-	public static final String PATH_DEFAULT= "/tmp/"; // TODO windows default...?
+	public static final String PATH_DEFAULT = defaultCachePath();
 	public static final boolean BYDEFAULT= false;
 
+	private static final Set<PosixFilePermission> NON_OWNER_PERMISSIONS = EnumSet.of(
+		PosixFilePermission.GROUP_READ, PosixFilePermission.GROUP_WRITE, PosixFilePermission.GROUP_EXECUTE,
+		PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_WRITE, PosixFilePermission.OTHERS_EXECUTE
+	);
 
 	public static String path = PATH_DEFAULT;
 	public static Boolean enabled = false;
@@ -52,9 +67,13 @@ public static synchronized SimpleCache createSimpleCache(){
 				t.printStackTrace();
 				Dump.dumpWithException(t);
 			}
-			File f = new File(path);
-			if (!f.exists()){
-				f.mkdir();
+			if (!preparePrivateDirectory(path)) {
+				System.err.println(
+					"Disabling the weaving cache: " + path + " is not a directory that only the current user can write to. " +
+					"Point " + CACHE_DIR + " at a private directory."
+				);
+				enabled = false;
+				return null;
 			}
 			lacache= new SimpleCache(path, enabled);
 		}
@@ -100,5 +119,72 @@ public static boolean isEnabled() {
 		return enabled;
 	}
 
+	/**
+	 * Per user directory below the JVM temporary directory. The system temporary directory itself is shared between all
+	 * local users on most platforms, and the cache index below it is read back with {@link java.io.ObjectInputStream}
+	 * while the cached bytes are handed to {@code ClassLoader.defineClass}.
+	 */
+	private static String defaultCachePath() {
+		StringBuilder name = new StringBuilder("aspectj-cache-");
+		String user = System.getProperty("user.name", "");
+		for (int i = 0; i < user.length(); i++) {
+			char c = user.charAt(i);
+			name.append(Character.isLetterOrDigit(c) ? c : '_');
+		}
+		return new File(System.getProperty("java.io.tmpdir", "."), name.toString()).getPath();
+	}
+
+	/**
+	 * Create the cache directory if it is missing and check that nobody but its owner can write to it. A directory a
+	 * second local user can write to lets that user replace the cached bytes of any class, which the weaver then
+	 * defines in this JVM.
+	 *
+	 * @param path cache directory to create and check
+	 * @return true if the directory exists and is private to its owner
+	 */
+	static boolean preparePrivateDirectory(String path) {
+		try {
+			Path dir = Paths.get(path);
+			if (!Files.isDirectory(dir, LinkOption.NOFOLLOW_LINKS)) {
+				createPrivateDirectory(dir);
+			}
+			if (!Files.isDirectory(dir, LinkOption.NOFOLLOW_LINKS)) {
+				return false;
+			}
+			Set<PosixFilePermission> permissions = readPosixPermissions(dir);
+			return permissions == null || Collections.disjoint(permissions, NON_OWNER_PERMISSIONS);
+		} catch (IOException | RuntimeException e) {
+			return false;
+		}
+	}
+
+	private static void createPrivateDirectory(Path dir) throws IOException {
+		Path parent = dir.getParent();
+		if (parent != null && !Files.isDirectory(parent)) {
+			Files.createDirectories(parent);
+		}
+		try {
+			Files.createDirectory(dir, PosixFilePermissions.asFileAttribute(
+				EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE)
+			));
+		} catch (UnsupportedOperationException e) {
+			// no POSIX file attributes, e.g. on Windows
+			Files.createDirectory(dir);
+		} catch (FileAlreadyExistsException e) {
+			// created concurrently, the permission check below still applies
+		}
+	}
+
+	/**
+	 * @return the POSIX permissions of the given directory, or null on a file system that does not report them
+	 */
+	private static Set<PosixFilePermission> readPosixPermissions(Path dir) throws IOException {
+		try {
+			return Files.getPosixFilePermissions(dir, LinkOption.NOFOLLOW_LINKS);
+		} catch (UnsupportedOperationException e) {
+			return null;
+		}
+	}
+
 
 }

weaver/src/test/java/org/aspectj/weaver/WeaverModuleTests.java

@@ -62,6 +62,7 @@
 import org.aspectj.weaver.tools.cache.DefaultCacheKeyResolverTest;
 import org.aspectj.weaver.tools.cache.DefaultFileCacheBackingTest;
 import org.aspectj.weaver.tools.cache.FlatFileCacheBackingTest;
+import org.aspectj.weaver.tools.cache.SimpleCacheFactoryTest;
 import org.aspectj.weaver.tools.cache.SimpleClassCacheTest;
 import org.aspectj.weaver.tools.cache.WeavedClassCacheTest;
 import org.aspectj.weaver.tools.cache.ZippedFileCacheBackingTest;
@@ -153,6 +154,7 @@ public static Test suite() {
 		suite.addTestSuite(DefaultCacheKeyResolverTest.class);
 		suite.addTestSuite(DefaultFileCacheBackingTest.class);
 		suite.addTestSuite(FlatFileCacheBackingTest.class);
+		suite.addTestSuite(SimpleCacheFactoryTest.class);
 		suite.addTestSuite(SimpleClassCacheTest.class);
 		suite.addTestSuite(WeavedClassCacheTest.class);
 		suite.addTestSuite(ZippedFileCacheBackingTest.class);

weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleCacheFactoryTest.java

@@ -0,0 +1,75 @@
+/*******************************************************************************
+ * Copyright (c) 2024 Contributors.
+ * All rights reserved.
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Public License v 2.0
+ * which accompanies this distribution and is available at
+ * https://www.eclipse.org/org/documents/epl-2.0/EPL-2.0.txt
+ ********************************************************************************/
+
+package org.aspectj.weaver.tools.cache;
+
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.Set;
+
+import junit.framework.TestCase;
+
+public class SimpleCacheFactoryTest extends TestCase {
+
+	private static boolean isPosix() {
+		return FileSystems.getDefault().supportedFileAttributeViews().contains("posix");
+	}
+
+	public void testCreatesDirectoryPrivateToTheOwner() throws Exception {
+		if (!isPosix()) {
+			return;
+		}
+		Path parent = Files.createTempDirectory("ajCacheTest");
+		Path cacheDir = parent.resolve("cache");
+
+		assertTrue("a missing cache directory should be created", SimpleCacheFactory.preparePrivateDirectory(cacheDir.toString()));
+
+		Set<PosixFilePermission> permissions = Files.getPosixFilePermissions(cacheDir);
+		assertEquals("the cache directory should not be readable or writable by anybody else", "rwx------",
+			PosixFilePermissions.toString(permissions));
+	}
+
+	public void testRejectsWorldWritableDirectory() throws Exception {
+		if (!isPosix()) {
+			return;
+		}
+		Path cacheDir = Files.createTempDirectory("ajCacheTest");
+		Files.setPosixFilePermissions(cacheDir, PosixFilePermissions.fromString("rwxrwxrwx"));
+
+		assertFalse("a cache directory any local user can write to should be rejected",
+			SimpleCacheFactory.preparePrivateDirectory(cacheDir.toString()));
+
+		Files.setPosixFilePermissions(cacheDir, PosixFilePermissions.fromString("rwx------"));
+		assertTrue("a cache directory private to its owner should be accepted",
+			SimpleCacheFactory.preparePrivateDirectory(cacheDir.toString()));
+	}
+
+	public void testRejectsSymbolicLink() throws Exception {
+		if (!isPosix()) {
+			return;
+		}
+		Path parent = Files.createTempDirectory("ajCacheTest");
+		Path target = Files.createDirectory(parent.resolve("target"));
+		Path link = Files.createSymbolicLink(parent.resolve("link"), target);
+
+		assertFalse("a symbolic link should not be used as the cache directory",
+			SimpleCacheFactory.preparePrivateDirectory(link.toString()));
+	}
+
+	public void testDefaultPathIsNotTheSharedTemporaryDirectory() {
+		String tmpDir = System.getProperty("java.io.tmpdir");
+		assertFalse("the default cache directory should not be the shared temporary directory itself",
+			tmpDir.equals(SimpleCacheFactory.PATH_DEFAULT) || (tmpDir + "/").equals(SimpleCacheFactory.PATH_DEFAULT));
+		assertTrue("the default cache directory should live below the temporary directory",
+			SimpleCacheFactory.PATH_DEFAULT.startsWith(tmpDir));
+	}
+}