diff --git a/weaver/src/main/java/org/aspectj/weaver/tools/cache/AbstractIndexedFileCacheBacking.java b/weaver/src/main/java/org/aspectj/weaver/tools/cache/AbstractIndexedFileCacheBacking.java index e32ffcafa..fe479dfd7 100644 --- a/weaver/src/main/java/org/aspectj/weaver/tools/cache/AbstractIndexedFileCacheBacking.java +++ b/weaver/src/main/java/org/aspectj/weaver/tools/cache/AbstractIndexedFileCacheBacking.java @@ -17,8 +17,11 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.InvalidClassException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.io.ObjectStreamClass; import java.io.Serializable; import java.io.StreamCorruptedException; import java.util.Arrays; @@ -128,7 +131,7 @@ public IndexEntry[] readIndex(File indexFile) { ObjectInputStream ois = null; try { - ois = new ObjectInputStream(new FileInputStream(indexFile)); + ois = new IndexInputStream(new FileInputStream(indexFile)); return (IndexEntry[]) ois.readObject(); } catch (Exception e) { if ((logger != null) && logger.isTraceEnabled()) { @@ -144,6 +147,36 @@ public IndexEntry[] readIndex(File indexFile) { return EMPTY_INDEX; } + /** + * The cache index file is only ever written as an {@link IndexEntry} array, but + * the file lives in a directory that may be shared between JVMs (or users) via + * the {@code aj.weaving.cache.dir} property. A tampered index file would otherwise + * turn {@link ObjectInputStream#readObject()} into an arbitrary deserialization + * sink, so limit the classes the stream is allowed to resolve to the index type. + */ + private static final class IndexInputStream extends ObjectInputStream { + private static final String ENTRY_TYPE = IndexEntry.class.getName(); + private static final String ENTRY_ARRAY_TYPE = IndexEntry[].class.getName(); + + IndexInputStream(InputStream in) throws IOException { + super(in); + } + + @Override + protected Class resolveClass(ObjectStreamClass desc) throws IOException, ClassNotFoundException { + String name = desc.getName(); + if (ENTRY_TYPE.equals(name) || ENTRY_ARRAY_TYPE.equals(name)) { + return super.resolveClass(desc); + } + throw new InvalidClassException(name, "unexpected class in cache index file"); + } + + @Override + protected Class resolveProxyClass(String[] interfaces) throws IOException, ClassNotFoundException { + throw new InvalidClassException("proxy", "unexpected proxy class in cache index file"); + } + } + protected void writeIndex (File indexFile, Map index) throws IOException { writeIndex(indexFile, LangUtil.isEmpty(index) ? Collections.emptyList() : index.values()); } diff --git a/weaver/src/test/java/org/aspectj/weaver/tools/cache/DefaultFileCacheBackingTest.java b/weaver/src/test/java/org/aspectj/weaver/tools/cache/DefaultFileCacheBackingTest.java index 2fea0af1b..61e45cfac 100644 --- a/weaver/src/test/java/org/aspectj/weaver/tools/cache/DefaultFileCacheBackingTest.java +++ b/weaver/src/test/java/org/aspectj/weaver/tools/cache/DefaultFileCacheBackingTest.java @@ -13,7 +13,13 @@ package org.aspectj.weaver.tools.cache; +import java.io.BufferedOutputStream; import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.io.Serializable; import java.util.zip.CRC32; import org.aspectj.util.LangUtil; @@ -158,6 +164,52 @@ public void testOriginalClassBytesChanged () { assertFalse("Cache file not removed", cachedFile.exists()); } + public void testReadIndexRejectsUnexpectedClass() throws Exception { + DefaultFileCacheBacking backing = DefaultFileCacheBacking.createBacking(root); + File indexFile = new File(root, AbstractIndexedFileCacheBacking.INDEX_FILE); + Gadget.deserialized = false; + writeObject(indexFile, new Gadget()); + + IndexEntry[] index = backing.readIndex(indexFile); + + assertTrue("Index should be empty after rejecting a hostile entry", LangUtil.isEmpty(index)); + assertFalse("Disallowed class was deserialized from the cache index", Gadget.deserialized); + } + + public void testReadIndexAcceptsIndexEntryArray() throws Exception { + DefaultFileCacheBacking backing = DefaultFileCacheBacking.createBacking(root); + File indexFile = new File(root, AbstractIndexedFileCacheBacking.INDEX_FILE); + IndexEntry ie = new IndexEntry(); + ie.key = fakeKey; + ie.crcClass = 42L; + writeObject(indexFile, new IndexEntry[] { ie }); + + IndexEntry[] index = backing.readIndex(indexFile); + + assertEquals(1, index.length); + assertEquals(fakeKey, index[0].key); + assertEquals(42L, index[0].crcClass); + } + + private static void writeObject(File file, Serializable object) throws IOException { + ObjectOutputStream oos = new ObjectOutputStream(new BufferedOutputStream(new FileOutputStream(file))); + try { + oos.writeObject(object); + } finally { + oos.close(); + } + } + + private static class Gadget implements Serializable { + private static final long serialVersionUID = 1L; + static boolean deserialized; + + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + deserialized = true; + } + } + private boolean indexEntryExists(AbstractIndexedFileCacheBacking cache, String key, long expectedCRC) throws Exception { long storedCRC = 0L; IndexEntry[] index = cache.readIndex(new File(root, AbstractIndexedFileCacheBacking.INDEX_FILE));