diff --git a/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCacheFactory.java b/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCacheFactory.java index 033e57b98..0d1c646a4 100644 --- a/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCacheFactory.java +++ b/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 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 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 readPosixPermissions(Path dir) throws IOException { + try { + return Files.getPosixFilePermissions(dir, LinkOption.NOFOLLOW_LINKS); + } catch (UnsupportedOperationException e) { + return null; + } + } + } diff --git a/weaver/src/test/java/org/aspectj/weaver/WeaverModuleTests.java b/weaver/src/test/java/org/aspectj/weaver/WeaverModuleTests.java index 5f36e7e80..9e48b5299 100644 --- a/weaver/src/test/java/org/aspectj/weaver/WeaverModuleTests.java +++ b/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); diff --git a/weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleCacheFactoryTest.java b/weaver/src/test/java/org/aspectj/weaver/tools/cache/SimpleCacheFactoryTest.java new file mode 100644 index 000000000..2deb5b3d1 --- /dev/null +++ b/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 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)); + } +}