From d946f3ccc9e12e418cdae11d7d91861f197a815e Mon Sep 17 00:00:00 2001 From: katherine-hough <32645020+katherine-hough@users.noreply.github.com> Date: Thu, 27 Jul 2023 09:26:59 -0400 Subject: [PATCH] Java 11 Fixes (#199) * * Fixed issue Unsafe#put/getObject methods in Java 11 causing JVM crash. * Refactored JLink packaging and Phosphor class patching into its * Removed double declaration of compiler plugin from pom.xml * Factored packing of Phosphor into java.base module out of PhosphorJLinkPlugin into new class PhosphorPacker * Factored patching of Phosphor class from Instrumenter into new class PhosphorPatcher * Added Java 11 to workflows * * Fixed Java 8 issue * * Hopefully fixed circular class loading dependency * * Hopefully fixed circular class loading dependency * * Split PhosphorStackFrame hashes into an upper part corresponding to the method name and a lower part corresponding to the return-less descriptor * Fixed tag loss related to VarHandles in Java 11 by patching the descriptor for the frame's hash (lower part) * * Split PhosphorStackFrame hashes into an upper part corresponding to the method name and a lower part corresponding to the return-less descriptor * Fixed tag loss related to VarHandles in Java 11 by patching the descriptor for the frame's hash (lower part) * * Increased version of checkout GHA in workflow to v3. * Added no -ntp (--no-transfer-progress) flag to maven commands in GHA workflow. * Fixed issue with Phosphor wrapper argument not being cleared for null values resulting in old values being passed. --- .github/workflows/main.yml | 8 +- .../psl/phosphor/BasicSourceSinkManager.java | 218 +++++++++--------- .../cs/psl/phosphor/Instrumenter.java | 153 +----------- .../cs/psl/phosphor/PhosphorPatcher.java | 171 ++++++++++++++ .../InstrumentedJREProxyGenerator.java | 9 +- .../instrumenter/ReflectionHidingMV.java | 7 + .../phosphor/instrumenter/TaintAdapter.java | 67 +++--- .../instrumenter/TaintMethodRecord.java | 1 + .../phosphor/instrumenter/TaintPassingMV.java | 32 +-- .../TaintTrackingClassVisitor.java | 15 +- .../instrumenter/UninstrumentedCompatMV.java | 2 +- .../phosphor/runtime/PhosphorStackFrame.java | 23 +- .../phosphor/runtime/TaintSourceWrapper.java | 2 +- .../instrumenter/PhosphorJLinkPlugin.java | 45 +--- .../phosphor/instrumenter/PhosphorPacker.java | 74 ++++++ pom.xml | 8 - 16 files changed, 466 insertions(+), 369 deletions(-) create mode 100644 Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/PhosphorPatcher.java create mode 100644 phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorPacker.java diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 27c80ee47..2fb1dfb68 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -11,15 +11,15 @@ jobs: fail-fast: false matrix: # java: [ '8', '11', '16', '17' ] - java: [ '8', '16' ] + java: [ '8', '11', '16' ] steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v3 - name: Build Phosphor - run: mvn -B -DskipTests install + run: mvn -B -ntp -DskipTests install - name: Setup java uses: actions/setup-java@v3 with: distribution: 'temurin' java-version: ${{ matrix.java }} - name: Run tests - run: mvn install -Ddacapo.skip=false + run: mvn install -ntp -Ddacapo.skip=false diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/BasicSourceSinkManager.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/BasicSourceSinkManager.java index 8416fbb9a..4b5cb0a18 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/BasicSourceSinkManager.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/BasicSourceSinkManager.java @@ -2,9 +2,8 @@ import edu.columbia.cs.psl.phosphor.runtime.NonModifiableClassException; import edu.columbia.cs.psl.phosphor.struct.SinglyLinkedList; -import edu.columbia.cs.psl.phosphor.struct.harmony.util.ConcurrentHashMap; -import edu.columbia.cs.psl.phosphor.struct.harmony.util.HashSet; -import edu.columbia.cs.psl.phosphor.struct.harmony.util.Set; +import edu.columbia.cs.psl.phosphor.struct.harmony.util.StringBuilder; +import edu.columbia.cs.psl.phosphor.struct.harmony.util.*; import org.objectweb.asm.tree.ClassNode; import java.io.ByteArrayInputStream; @@ -13,30 +12,21 @@ public class BasicSourceSinkManager extends SourceSinkManager { - private static ConcurrentHashMap sourceLabels = new ConcurrentHashMap<>(); - + private static final Map sourceLabels = new HashMap<>(); // Maps class names to a set of all the methods listed as sources for the class - private static ConcurrentHashMap> sources = new ConcurrentHashMap<>(); + private static final Map> sources = new HashMap<>(); // Maps class names to a set of all the methods listed as sinks for the class - private static ConcurrentHashMap> sinks = new ConcurrentHashMap<>(); - // Maps class names to a set of all the methods listed as taintThrough methods for the class - private static ConcurrentHashMap> taintThrough = new ConcurrentHashMap<>(); + private static final Map> sinks = new HashMap<>(); + // Maps class names to a set of all the methods listed as taintThrough methods for the class + private static final Map> taintThrough = new HashMap<>(); + // Maps class names to sets of class instances + private static final Map>> classMap = new HashMap<>(); // Maps class names to a set of all methods listed as sources for the class or one of its supertypes or superinterfaces - private static ConcurrentHashMap> inheritedSources = new ConcurrentHashMap<>(); + private static Map> inheritedSources = new HashMap<>(); // Maps class names to a set of all methods listed as sinks for the class or one of its supertypes or superinterfaces - private static ConcurrentHashMap> inheritedSinks = new ConcurrentHashMap<>(); + private static Map> inheritedSinks = new HashMap<>(); // Maps class names to a set of all methods listed as taintThrough methods for the class or one of its supertypes or superinterfaces - private static ConcurrentHashMap> inheritedTaintThrough = new ConcurrentHashMap<>(); - - // Maps class names to sets of class instances - private static ConcurrentHashMap>> classMap = new ConcurrentHashMap<>(); - - /* Reads source, sink and taintThrough methods from their files into their respective maps. */ - public static void loadTaintMethods() { - readTaintMethods(Instrumenter.sourcesFile, AutoTaint.SOURCE); - readTaintMethods(Instrumenter.sinksFile, AutoTaint.SINK); - readTaintMethods(Instrumenter.taintThroughFile, AutoTaint.TAINT_THROUGH); - } + private static Map> inheritedTaintThrough = new HashMap<>(); /* Private constructor ensures that only one instance of BasicSourceSinkManager is ever created. */ private BasicSourceSinkManager() { @@ -44,68 +34,84 @@ private BasicSourceSinkManager() { @Override public boolean isSourceOrSinkOrTaintThrough(Class clazz) { - if(clazz.getName() == null) { - return false; + synchronized (BasicSourceSinkManager.class) { + String className = clazz.getName().replace(".", "/"); + // This class has a sink, source or taintThrough method + return !getAutoTaintMethods(className, sinks, inheritedSinks).isEmpty() + || !getAutoTaintMethods(className, sources, inheritedSources).isEmpty() + || !getAutoTaintMethods(className, taintThrough, inheritedTaintThrough).isEmpty(); } - String className = clazz.getName().replace(".", "/"); - // This class has a sink, source or taintThrough method - return !getAutoTaintMethods(className, sinks, inheritedSinks).isEmpty() - || !getAutoTaintMethods(className, sources, inheritedSources).isEmpty() - || !getAutoTaintMethods(className, taintThrough, inheritedTaintThrough).isEmpty(); } @Override public Object getLabel(String str) { - return sourceLabels.get(str); + synchronized (BasicSourceSinkManager.class) { + return sourceLabels.get(str); + } } @Override public boolean isTaintThrough(String str) { - if(str.startsWith("[")) { - return false; - } else { - String[] parsed = str.split("\\."); - // Check if the set of taintThrough methods for the class name contains the method name - return getAutoTaintMethods(parsed[0], taintThrough, inheritedTaintThrough).contains(parsed[1]); + synchronized (BasicSourceSinkManager.class) { + if (str.startsWith("[")) { + return false; + } else { + String[] parsed = str.split("\\."); + // Check if the set of taintThrough methods for the class name contains the method name + return getAutoTaintMethods(parsed[0], taintThrough, inheritedTaintThrough).contains(parsed[1]); + } } } @Override public boolean isSource(String str) { - if(str.startsWith("[")) { - return false; - } else { - String[] parsed = str.split("\\."); - // Check if the set of source methods for the class name contains the method name - if(getAutoTaintMethods(parsed[0], sources, inheritedSources).contains(parsed[1])) { - String baseSource = findSuperTypeAutoTaintProvider(parsed[0], parsed[1], sources, inheritedSources); - if(!sourceLabels.containsKey(str)) { - sourceLabels.put(str, sourceLabels.get(String.format("%s.%s", baseSource, parsed[1]))); - } - return true; - } else { + synchronized (BasicSourceSinkManager.class) { + if (str.startsWith("[")) { return false; + } else { + String[] parsed = str.split("\\."); + // Check if the set of source methods for the class name contains the method name + if (getAutoTaintMethods(parsed[0], sources, inheritedSources).contains(parsed[1])) { + String baseSource = findSuperTypeAutoTaintProvider(parsed[0], parsed[1], sources, inheritedSources); + if (!sourceLabels.containsKey(str)) { + sourceLabels.put(str, sourceLabels.get(String.format("%s.%s", baseSource, parsed[1]))); + } + return true; + } else { + return false; + } } } } @Override public boolean isSink(String str) { - if(str.startsWith("[")) { - return false; - } else { - String[] parsed = str.split("\\."); - // Check if the set of sink methods for the class name contains the method name - return getAutoTaintMethods(parsed[0], sinks, inheritedSinks).contains(parsed[1]); + synchronized (BasicSourceSinkManager.class) { + if (str.startsWith("[")) { + return false; + } else { + String[] parsed = str.split("\\."); + // Check if the set of sink methods for the class name contains the method name + return getAutoTaintMethods(parsed[0], sinks, inheritedSinks).contains(parsed[1]); + } } } /* Returns the name of sink method from which the specified method inherited its sink property or null if the specified * method is not a sink. */ public String getBaseSink(String str) { - String[] parsed = str.split("\\."); - String baseSink = findSuperTypeAutoTaintProvider(parsed[0], parsed[1], sinks, inheritedSinks); - return baseSink == null ? null : String.format("%s.%s", baseSink, parsed[1]); + synchronized (BasicSourceSinkManager.class) { + String[] parsed = str.split("\\."); + String baseSink = findSuperTypeAutoTaintProvider(parsed[0], parsed[1], sinks, inheritedSinks); + return baseSink == null ? null : String.format("%s.%s", baseSink, parsed[1]); + } + } + + /* Reads source, sink and taintThrough methods from their files into their respective maps. */ + public static synchronized void loadTaintMethods() { + readTaintMethods(Instrumenter.sourcesFile, AutoTaint.SOURCE); + readTaintMethods(Instrumenter.sinksFile, AutoTaint.SINK); + readTaintMethods(Instrumenter.taintThroughFile, AutoTaint.TAINT_THROUGH); } /* Provides access to the single instance of BasicSourceSinkManager */ @@ -118,8 +124,8 @@ public static BasicSourceSinkManager getInstance() { private static synchronized void readTaintMethods(InputStream src, AutoTaint type) { Scanner s = null; String lastLine = null; - ConcurrentHashMap> baseMethods; - switch(type) { + Map> baseMethods; + switch (type) { case SOURCE: baseMethods = sources; break; @@ -130,31 +136,31 @@ private static synchronized void readTaintMethods(InputStream src, AutoTaint typ baseMethods = taintThrough; } try { - if(src != null) { + if (src != null) { s = new Scanner(src); - while(s.hasNextLine()) { + while (s.hasNextLine()) { String line = s.nextLine(); lastLine = line; - if(!line.startsWith("#") && !line.isEmpty()) { + if (!line.startsWith("#") && !line.isEmpty()) { String[] parsed = line.split("\\."); - if(!baseMethods.containsKey(parsed[0])) { + if (!baseMethods.containsKey(parsed[0])) { baseMethods.put(parsed[0], new HashSet()); } baseMethods.get(parsed[0]).add(parsed[1]); - if(type.equals(AutoTaint.SOURCE)) { + if (type.equals(AutoTaint.SOURCE)) { sourceLabels.put(line, line); } } } } - } catch(Throwable e) { + } catch (Throwable e) { System.err.printf("Unable to parse %s file: %s\n", type.name, src); - if(lastLine != null) { + if (lastLine != null) { System.err.printf("Last line read: '%s'\n", lastLine); } throw new RuntimeException(e); } finally { - if(s != null) { + if (s != null) { s.close(); } } @@ -163,16 +169,16 @@ private static synchronized void readTaintMethods(InputStream src, AutoTaint typ /* Stores the specified class instance so that it can last be used to retransform the class if it's autoTaint methods * change as a result of a call to replaceAutoTaintMethods. */ public static synchronized void recordClass(Class clazz) { - if(clazz.getName() != null) { - String key = clazz.getName().replace(".", "/"); - classMap.putIfAbsent(key, new HashSet<>()); - classMap.get(key).add(clazz); + String key = clazz.getName().replace(".", "/"); + if (!classMap.containsKey(key)) { + classMap.put(key, new HashSet<>()); } + classMap.get(key).add(clazz); } public static synchronized java.util.LinkedList replaceAutoTaintMethods(Iterable src, AutoTaint type) { StringBuilder builder = new StringBuilder(); - for(String s : src) { + for (String s : src) { builder.append(s).append("\n"); } return replaceAutoTaintMethods(new ByteArrayInputStream(builder.toString().getBytes()), type); @@ -182,35 +188,35 @@ public static synchronized java.util.LinkedList replaceAutoTaintMethods( * any class with a method whose status as an autoTaint methods of the specified type has changed. Returns a list of * the replaced base autoTaint methods of the specified type. */ public static synchronized java.util.LinkedList replaceAutoTaintMethods(InputStream src, AutoTaint type) { - ConcurrentHashMap> baseMethods; - ConcurrentHashMap> inheritedMethods; - ConcurrentHashMap> prevInheritedMethods; - switch(type) { + Map> baseMethods; + Map> inheritedMethods; + Map> prevInheritedMethods; + switch (type) { case SOURCE: baseMethods = sources; prevInheritedMethods = inheritedSources; // Clear the map of inherited or derived autoTaint methods of the specified type - inheritedMethods = new ConcurrentHashMap<>(); + inheritedMethods = new HashMap<>(); inheritedSources = inheritedMethods; break; case SINK: baseMethods = sinks; prevInheritedMethods = inheritedSinks; // Clear the map of inherited or derived autoTaint methods of the specified type - inheritedMethods = new ConcurrentHashMap<>(); + inheritedMethods = new HashMap<>(); inheritedSinks = inheritedMethods; break; default: baseMethods = taintThrough; prevInheritedMethods = inheritedTaintThrough; // Clear the map of inherited or derived autoTaint methods of the specified type - inheritedMethods = new ConcurrentHashMap<>(); + inheritedMethods = new HashMap<>(); inheritedTaintThrough = inheritedMethods; } // Reconstruct the original set of base methods java.util.LinkedList prevBaseMethods = new java.util.LinkedList<>(); - for(String className : baseMethods.keySet()) { - for(String methodName : baseMethods.get(className)) { + for (String className : baseMethods.keySet()) { + for (String methodName : baseMethods.get(className)) { prevBaseMethods.add(className + "." + methodName); } } @@ -219,19 +225,19 @@ public static synchronized java.util.LinkedList replaceAutoTaintMethods( readTaintMethods(src, type); // Retransform any class that has a method that changed from being a autoTaint methods of the specified type // to a not being an autoTaint methods of the specified type or vice versa - for(String className : prevInheritedMethods.keySet()) { + for (String className : prevInheritedMethods.keySet()) { Set autoTaintMethods = getAutoTaintMethods(className, baseMethods, inheritedMethods); - if(!autoTaintMethods.equals(prevInheritedMethods.get(className))) { + if (!autoTaintMethods.equals(prevInheritedMethods.get(className))) { // Set of autoTaint methods for this class changed try { - if(classMap.containsKey(className)) { - for(Class clazz : classMap.get(className)) { + if (classMap.containsKey(className)) { + for (Class clazz : classMap.get(className)) { PreMain.getInstrumentationHelper().retransformClasses(clazz); } } - } catch(NonModifiableClassException e) { + } catch (NonModifiableClassException e) { // - } catch(Throwable t) { + } catch (Throwable t) { // Make sure that any other type of exception is printed t.printStackTrace(); throw t; @@ -245,27 +251,27 @@ public static synchronized java.util.LinkedList replaceAutoTaintMethods( * the class or interface with the specified slash-separated string class name. A method is considered to be an auto * taint method if the method is present in the set of base auto taint methods for either the specified class or a supertype of the * specified class. Previously determined auto taint methods are stored in inheritedMethods. */ - private static synchronized Set getAutoTaintMethods(String className, ConcurrentHashMap> baseMethods, - ConcurrentHashMap> inheritedMethods) { - if(inheritedMethods.containsKey(className)) { + private static synchronized Set getAutoTaintMethods(String className, Map> baseMethods, + Map> inheritedMethods) { + if (inheritedMethods.containsKey(className)) { // The auto taint methods for this class have already been determined. return inheritedMethods.get(className); } else { // Recursively build the set of auto taint methods for this class Set set = new HashSet<>(); - if(baseMethods.containsKey(className)) { + if (baseMethods.containsKey(className)) { // Add any methods from this class that are directly listed as auto taint methods set.addAll(baseMethods.get(className)); } ClassNode cn = Instrumenter.getClassNode(className); - if(cn != null) { - if(cn.interfaces != null) { + if (cn != null) { + if (cn.interfaces != null) { // Add all auto taint methods from interfaces implemented by this class - for(Object inter : cn.interfaces) { - set.addAll(getAutoTaintMethods((String) inter, baseMethods, inheritedMethods)); + for (String inter : cn.interfaces) { + set.addAll(getAutoTaintMethods(inter, baseMethods, inheritedMethods)); } } - if(cn.superName != null && !cn.superName.equals("java/lang/Object")) { + if (cn.superName != null && !cn.superName.equals("java/lang/Object")) { // Add all auto taint methods from the superclass of this class set.addAll(getAutoTaintMethods(cn.superName, baseMethods, inheritedMethods)); } @@ -278,26 +284,26 @@ private static synchronized Set getAutoTaintMethods(String className, Co /* Returns the string class name of the supertype of the class or interface with specified string class name from which * its method with the specified method name derived its status as an auto taint (i.e. source, sink or taintThrough) * method. */ - private static synchronized String findSuperTypeAutoTaintProvider(String className, String methodName, ConcurrentHashMap> baseMethods, ConcurrentHashMap> inheritedMethods) { + private static synchronized String findSuperTypeAutoTaintProvider(String className, String methodName, Map> baseMethods, Map> inheritedMethods) { SinglyLinkedList queue = new SinglyLinkedList<>(); queue.enqueue(className); - while(!queue.isEmpty()) { + while (!queue.isEmpty()) { String curClassName = queue.pop(); // Check that the current class actually has an inherited auto taint method with the target method name - if(inheritedMethods.containsKey(curClassName) && inheritedMethods.get(curClassName).contains(methodName)) { - if(baseMethods.containsKey(curClassName) && baseMethods.get(curClassName).contains(methodName)) { + if (inheritedMethods.containsKey(curClassName) && inheritedMethods.get(curClassName).contains(methodName)) { + if (baseMethods.containsKey(curClassName) && baseMethods.get(curClassName).contains(methodName)) { return curClassName; } ClassNode cn = Instrumenter.getClassNode(curClassName); - if(cn != null) { - if(cn.interfaces != null) { + if (cn != null) { + if (cn.interfaces != null) { // Enqueue interfaces implemented by the current class - for(Object inter : cn.interfaces) { - queue.enqueue((String) inter); + for (String inter : cn.interfaces) { + queue.enqueue(inter); } } - if(cn.superName != null && !cn.superName.equals("java/lang/Object")) { + if (cn.superName != null && !cn.superName.equals("java/lang/Object")) { // Enqueue the superclass of the current class queue.enqueue(cn.superName); } diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/Instrumenter.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/Instrumenter.java index be641fa74..71f023c72 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/Instrumenter.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/Instrumenter.java @@ -1,18 +1,14 @@ package edu.columbia.cs.psl.phosphor; -import edu.columbia.cs.psl.phosphor.instrumenter.ConfigurationEmbeddingMV; import edu.columbia.cs.psl.phosphor.instrumenter.TaintTrackingClassVisitor; import edu.columbia.cs.psl.phosphor.runtime.StringUtils; -import edu.columbia.cs.psl.phosphor.runtime.jdk.unsupported.UnsafeProxy; import edu.columbia.cs.psl.phosphor.struct.harmony.util.*; import org.apache.commons.cli.CommandLine; -import org.objectweb.asm.*; -import org.objectweb.asm.commons.ModuleHashesAttribute; -import org.objectweb.asm.commons.ModuleResolutionAttribute; -import org.objectweb.asm.commons.ModuleTargetAttribute; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.tree.ClassNode; import org.objectweb.asm.tree.MethodNode; -import org.objectweb.asm.tree.ModuleExportNode; import java.io.*; import java.lang.instrument.ClassFileTransformer; @@ -709,148 +705,6 @@ private static class Result { byte[] buf; } - public static byte[] transformJavaBaseModuleInfo(InputStream is, java.util.Collection packages) throws IOException { - ClassNode classNode = new ClassNode(); - ClassReader cr = new ClassReader(is); - java.util.List attrs = new java.util.ArrayList<>(); - attrs.add(new ModuleTargetAttribute()); - attrs.add(new ModuleResolutionAttribute()); - attrs.add(new ModuleHashesAttribute()); - - cr.accept(classNode, attrs.toArray(new Attribute[0]), 0); - //Add export - classNode.module.exports.add(new ModuleExportNode("edu/columbia/cs/psl/phosphor", 0, null)); - classNode.module.exports.add(new ModuleExportNode("edu/columbia/cs/psl/phosphor/control", 0, null)); - classNode.module.exports.add(new ModuleExportNode("edu/columbia/cs/psl/phosphor/control/standard", 0, null)); - classNode.module.exports.add(new ModuleExportNode("edu/columbia/cs/psl/phosphor/runtime", 0, null)); - classNode.module.exports.add(new ModuleExportNode("edu/columbia/cs/psl/phosphor/struct", 0, null)); - - //Add pac - classNode.module.packages.addAll(packages); - ClassWriter cw = new ClassWriter(0); - classNode.accept(cw); - return cw.toByteArray(); - } - - public static boolean isPhosphorClassPatchedAtInstTime(String name){ - return name.equals("edu/columbia/cs/psl/phosphor/Configuration.class") || name.equals("edu/columbia/cs/psl/phosphor/runtime/RuntimeJDKInternalUnsafePropagator.class"); - } - - /** - * We do rewriting of various phosphor classes to ensure easy compilation for java < 9 - */ - public static byte[] patchPhosphorClass(String name, InputStream is) throws IOException { - if(name.equals("edu/columbia/cs/psl/phosphor/Configuration.class")){ - return embedPhopshorConfiguration(is); - } - if (name.equals("edu/columbia/cs/psl/phosphor/runtime/RuntimeJDKInternalUnsafePropagator.class")) { - return transformRuntimeUnsafePropagator(is, "jdk/internal/misc/Unsafe"); - } - throw new UnsupportedEncodingException("We do not plan to instrument " + name); - } - - public static byte[] transformRuntimeUnsafePropagator(InputStream is, String targetUnsafeInternalName) throws IOException { - final String UNSAFE_PROXY_INTERNAL_NAME = Type.getInternalName(UnsafeProxy.class); - final String UNSAFE_PROXY_DESC = Type.getDescriptor(UnsafeProxy.class); - final String TARGET_UNSAFE_INTERNAL_NAME = targetUnsafeInternalName; - final String TARGET_UNSAFE_DESC = "L" + targetUnsafeInternalName + ";"; - ClassReader cr = new ClassReader(is); - ClassWriter cw = new ClassWriter(cr, 0); - ClassVisitor cv = new ClassVisitor(Opcodes.ASM9, cw) { - private String patchInternalName(String in) { - if (in.equals(UNSAFE_PROXY_INTERNAL_NAME)) { - return TARGET_UNSAFE_INTERNAL_NAME; - } - return in; - } - - private String patchDesc(String in) { - if (in == null) { - return null; - } - return in.replace(UNSAFE_PROXY_DESC, TARGET_UNSAFE_DESC); - } - - @Override - public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { - MethodVisitor mv = super.visitMethod(access, name, patchDesc(descriptor), patchDesc(signature), exceptions); - return new MethodVisitor(Opcodes.ASM9, mv) { - - @Override - public void visitFrame(int type, int numLocal, Object[] local, int numStack, Object[] stack) { - for (int i = 0; i < numLocal; i++) { - if (local[i] instanceof String) { - local[i] = patchInternalName((String) local[i]); - } - } - for (int i = 0; i < numStack; i++) { - if (stack[i] instanceof String) { - stack[i] = patchInternalName((String) stack[i]); - } - } - super.visitFrame(type, numLocal, local, numStack, stack); - } - - @Override - public void visitFieldInsn(int opcode, String owner, String name, String descriptor) { - super.visitFieldInsn(opcode, patchInternalName(owner), name, patchDesc(descriptor)); - } - - @Override - public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) { - super.visitMethodInsn(opcode, patchInternalName(owner), name, patchDesc(descriptor), isInterface); - } - - @Override - public void visitTypeInsn(int opcode, String type) { - super.visitTypeInsn(opcode, patchInternalName(type)); - } - - @Override - public void visitLocalVariable(String name, String descriptor, String signature, Label start, Label end, int index) { - super.visitLocalVariable(name, patchDesc(descriptor), signature, start, end, index); - } - }; - } - }; - cr.accept(cv, 0); - return cw.toByteArray(); - - } - /** - * To boot the JVM... we need to have this flag set correctly. - * - * Default to setting it to be Java 8. - * - * In Java 9+, we pack Phosphor into the java.base module, and rewrite the configuration file - * to set the flag to false. - * - * @param is - * @return - * @throws IOException - */ - public static byte[] embedPhopshorConfiguration(InputStream is) throws IOException { - ClassReader cr = new ClassReader(is); - ClassWriter cw = new ClassWriter(cr, 0); - - ClassVisitor cv = new ClassVisitor(Opcodes.ASM9, cw) { - @Override - public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { - MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); - if (name.equals("")) { - return new ConfigurationEmbeddingMV(mv); - } else { - return mv; - } - } - }; - - cr.accept(cv, 0); - return cw.toByteArray(); - - } - - public static boolean isJava8JVMDir(File java_home) { return new File(java_home, "bin" + File.separator + "java").exists() && !new File(java_home, "jmods").exists() @@ -861,5 +715,4 @@ public static boolean isUnsafeClass(String className){ return (Configuration.IS_JAVA_8 && "sun/misc/Unsafe".equals(className)) || (!Configuration.IS_JAVA_8 && "jdk/internal/misc/Unsafe".equals(className)); } - } diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/PhosphorPatcher.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/PhosphorPatcher.java new file mode 100644 index 000000000..49bd76f3f --- /dev/null +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/PhosphorPatcher.java @@ -0,0 +1,171 @@ +package edu.columbia.cs.psl.phosphor; + +import edu.columbia.cs.psl.phosphor.instrumenter.ConfigurationEmbeddingMV; +import edu.columbia.cs.psl.phosphor.runtime.jdk.unsupported.UnsafeProxy; +import org.objectweb.asm.*; +import org.objectweb.asm.commons.ModuleHashesAttribute; +import org.objectweb.asm.commons.ModuleResolutionAttribute; +import org.objectweb.asm.commons.ModuleTargetAttribute; +import org.objectweb.asm.tree.ClassNode; +import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.ModuleExportNode; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Set; + +public class PhosphorPatcher { + private final boolean patchUnsafeNames; + + public PhosphorPatcher(InputStream unsafeContent) throws IOException { + this.patchUnsafeNames = shouldPatchUnsafeNames(unsafeContent); + } + + public byte[] patch(String name, byte[] content) throws IOException { + if (name.equals("edu/columbia/cs/psl/phosphor/Configuration.class")) { + return setConfigurationVersion(new ByteArrayInputStream(content)); + } else if (name.equals("edu/columbia/cs/psl/phosphor/runtime/RuntimeJDKInternalUnsafePropagator.class")) { + return transformUnsafePropagator(new ByteArrayInputStream(content), + "jdk/internal/misc/Unsafe", patchUnsafeNames); + } else { + return content; + } + } + + public byte[] transformBaseModuleInfo(InputStream in, Set packages) { + try { + ClassNode classNode = new ClassNode(); + ClassReader cr = new ClassReader(in); + Attribute[] attributes = new Attribute[]{new ModuleTargetAttribute(), + new ModuleResolutionAttribute(), + new ModuleHashesAttribute()}; + cr.accept(classNode, attributes, 0); + // Add exports + for (String packageName : packages) { + classNode.module.exports.add(new ModuleExportNode(packageName, 0, null)); + } + // Add packages + classNode.module.packages.addAll(packages); + ClassWriter cw = new ClassWriter(0); + classNode.accept(cw); + return cw.toByteArray(); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static boolean shouldPatchUnsafeNames(InputStream in) throws IOException { + ClassNode cn = new ClassNode(); + new ClassReader(in).accept(cn, 0); + for (MethodNode mn : cn.methods) { + if (mn.name.contains("putReference")) { + return false; + } + } + return true; + } + + /** + * Modify {@link Configuration} to set {@link Configuration#IS_JAVA_8} to false. + * This flag must be set correctly in order to boot the JVM. + */ + private static byte[] setConfigurationVersion(InputStream is) throws IOException { + ClassReader cr = new ClassReader(is); + ClassWriter cw = new ClassWriter(cr, 0); + ClassVisitor cv = new ClassVisitor(Configuration.ASM_VERSION, cw) { + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, + String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions); + if (name.equals("")) { + return new ConfigurationEmbeddingMV(mv); + } else { + return mv; + } + } + }; + cr.accept(cv, 0); + return cw.toByteArray(); + } + + public static byte[] transformUnsafePropagator(InputStream in, String unsafeInternalName, + boolean patchUnsafeNames) throws IOException { + ClassReader cr = new ClassReader(in); + ClassWriter cw = new ClassWriter(cr, 0); + ClassVisitor cv = new UnsafePatchingCV(cw, unsafeInternalName, patchUnsafeNames); + cr.accept(cv, 0); + return cw.toByteArray(); + } + + private static class UnsafePatchingCV extends ClassVisitor { + private static final String UNSAFE_PROXY_INTERNAL_NAME = Type.getInternalName(UnsafeProxy.class); + private static final String UNSAFE_PROXY_DESC = Type.getDescriptor(UnsafeProxy.class); + private final String unsafeDesc; + private final boolean patchNames; + private final String unsafeInternalName; + + public UnsafePatchingCV(ClassWriter cw, String unsafeInternalName, boolean patchNames) { + super(Configuration.ASM_VERSION, cw); + this.unsafeInternalName = unsafeInternalName; + this.unsafeDesc = "L" + unsafeInternalName + ";"; + this.patchNames = patchNames; + } + + private String patchInternalName(String name) { + return UNSAFE_PROXY_INTERNAL_NAME.equals(name) ? unsafeInternalName : name; + } + + private String patchDesc(String desc) { + return desc == null ? null : desc.replace(UNSAFE_PROXY_DESC, unsafeDesc); + } + + private String patchMethodName(String owner, String name) { + return patchNames && owner.equals(UNSAFE_PROXY_INTERNAL_NAME) ? + name.replace("Reference", "Object") : name; + } + + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { + MethodVisitor mv = super.visitMethod(access, name, patchDesc(descriptor), patchDesc(signature), exceptions); + return new MethodVisitor(api, mv) { + @Override + public void visitFrame(int type, int numLocal, Object[] local, int numStack, Object[] stack) { + for (int i = 0; i < numLocal; i++) { + if (local[i] instanceof String) { + local[i] = patchInternalName((String) local[i]); + } + } + for (int i = 0; i < numStack; i++) { + if (stack[i] instanceof String) { + stack[i] = patchInternalName((String) stack[i]); + } + } + super.visitFrame(type, numLocal, local, numStack, stack); + } + + @Override + public void visitFieldInsn(int opcode, String owner, String name, String descriptor) { + super.visitFieldInsn(opcode, patchInternalName(owner), name, patchDesc(descriptor)); + } + + @Override + public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) { + super.visitMethodInsn(opcode, patchInternalName(owner), patchMethodName(owner, name), + patchDesc(descriptor), isInterface); + } + + @Override + public void visitTypeInsn(int opcode, String type) { + super.visitTypeInsn(opcode, patchInternalName(type)); + } + + @Override + public void visitLocalVariable(String name, String descriptor, String signature, Label start, Label end, + int index) { + super.visitLocalVariable(name, patchDesc(descriptor), signature, start, end, index); + } + }; + } + } +} diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/InstrumentedJREProxyGenerator.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/InstrumentedJREProxyGenerator.java index bb6dff475..7a8a988a6 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/InstrumentedJREProxyGenerator.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/InstrumentedJREProxyGenerator.java @@ -2,12 +2,14 @@ import edu.columbia.cs.psl.phosphor.Configuration; -import edu.columbia.cs.psl.phosphor.Instrumenter; +import edu.columbia.cs.psl.phosphor.PhosphorPatcher; import org.objectweb.asm.*; import org.objectweb.asm.commons.GeneratorAdapter; import org.objectweb.asm.util.CheckClassAdapter; -import java.io.*; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Paths; @@ -21,7 +23,8 @@ public static void main(String[] args) throws IOException { String pathToUnsafePropagator = outputDir + "/edu/columbia/cs/psl/phosphor/runtime/jdk/unsupported/RuntimeSunMiscUnsafePropagator.class"; InputStream sunMiscUnsafeIn = new FileInputStream(pathToUnsafePropagator); - byte[] instrumentedUnsafe = Instrumenter.transformRuntimeUnsafePropagator(sunMiscUnsafeIn, "sun/misc/Unsafe"); + byte[] instrumentedUnsafe = PhosphorPatcher.transformUnsafePropagator(sunMiscUnsafeIn, + "sun/misc/Unsafe", false); Files.write(Paths.get(pathToUnsafePropagator), instrumentedUnsafe); for (String clazz : CLASSES) { diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/ReflectionHidingMV.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/ReflectionHidingMV.java index 877533320..c4773c39d 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/ReflectionHidingMV.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/ReflectionHidingMV.java @@ -159,6 +159,8 @@ private boolean isUnsafeIntrinsic(String owner, String name, String desc) { if (!Instrumenter.isUnsafeClass(owner)) { return false; } + // Java 11 uses get/putObject instead of Reference + name = name.replace("Object", "Reference"); switch (desc) { case "(Ljava/lang/Object;JLjava/lang/Object;)V": switch (name) { @@ -517,6 +519,10 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, } } desc = "(L" + owner + ";" + desc.substring(1); + if (isUnsafeIntrinsic(owner, name, descWithoutStackFrame)) { + // Java 11 uses get/putObject instead of Reference + name = name.replace("Object", "Reference"); + } super.visitMethodInsn(Opcodes.INVOKESTATIC, getRuntimeUnsafePropogatorClassName(), name, desc, false); return; } else if (isUnsafeFieldSetter(opcode, owner, name, args, nameWithoutSuffix)) { @@ -628,6 +634,7 @@ private static boolean isUnsafeReferenceFieldGetter(String methodName) { if (Configuration.IS_JAVA_8) { return "getObject".equals(methodName) || "getObjectVolatile".equals(methodName); } + // TODO Java 11? return "getReference".equals(methodName) || "getReferenceVolatile".equals(methodName); } } diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintAdapter.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintAdapter.java index cd286bcc7..09e5b07d8 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintAdapter.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintAdapter.java @@ -734,6 +734,12 @@ public void unwrapArraysForCallTo(String owner, String name, String desc){ super.visitInsn(POP); super.visitInsn(ACONST_NULL); } + } else if(TaintUtils.isWrappedType(expected)) { + // Clear the arg wrapper + pushPhosphorStackFrame(); + super.visitInsn(ACONST_NULL); + push(idxOfThisArg); + SET_ARG_WRAPPER.delegateVisit(mv); } super.visitVarInsn(t.getOpcode(ISTORE), tmp[i]); } @@ -742,11 +748,14 @@ public void unwrapArraysForCallTo(String owner, String name, String desc){ lvs.freeTmpLV(tmp[i]); } } - protected static String getMethodKeyForStackFrame(String name, String desc, boolean polymorphicSignatureMethod) { + protected static String getMethodKeyForStackFrame(String name, String desc, + boolean polymorphicSignatureMethod) { + return name + getStackFrameDesc(desc, polymorphicSignatureMethod); + } + + private static String getStackFrameDesc(String desc, boolean polymorphicSignatureMethod) { if (polymorphicSignatureMethod) { - StringBuilder sig = new StringBuilder(); - sig.append(name); - sig.append('('); + StringBuilder sig = new StringBuilder("("); Type[] args = Type.getArgumentTypes(desc); for (Type t : args) { if (t.getSort() == Type.ARRAY) { @@ -755,28 +764,38 @@ protected static String getMethodKeyForStackFrame(String name, String desc, bool sig.append(t.getDescriptor()); } } - sig.append(')'); - return sig.toString(); + return sig.append(')').toString(); + } else { + return desc.substring(0, 1 + desc.indexOf(')')); } - return name + desc.substring(0, 1 + desc.indexOf(')')); } - protected void prepareMetadataLocalVariables(){ - if (this.initializePhosphorStackFrame) { - //There are methods that the JDK will internally resolve to, and prefix the arguments - //with a handle to the underlying object. That handle won't be passed explicitly by the caller. - String descForStackFrame = this.methodDesc; - if (this.className.startsWith("java/lang/invoke/VarHandleReferences")) { - if (this.methodDesc.contains("java/lang/invoke/VarHandle")) { - descForStackFrame = this.methodDesc.replace("Ljava/lang/invoke/VarHandle;", ""); - } + protected static int getHashForStackFrame(String name, String desc, boolean polymorphicSignatureMethod) { + return PhosphorStackFrame.computeFrameHash(name, getStackFrameDesc(desc, polymorphicSignatureMethod)); + } + + protected static String fixDescForStack(String className, String methodDesc) { + String descForStackFrame = methodDesc; + if (className.startsWith("java/lang/invoke/VarHandle")) { + // There are methods that the JDK will internally resolve to and prefix the arguments + // with a handle to the underlying object. + // Remove the parameter for the handle to the underlying object as the caller does not + // explicitly pass the handle. + if (methodDesc.startsWith("(Ljava/lang/invoke/VarHandle")) { + descForStackFrame = "(" + methodDesc.substring(methodDesc.indexOf(";") + 1); } + } + return descForStackFrame; + } + protected void prepareMetadataLocalVariables(){ + if (this.initializePhosphorStackFrame) { + String descForStackFrame = fixDescForStack(className, methodDesc); if (Configuration.DEBUG_STACK_FRAME_WRAPPERS) { - super.visitLdcInsn(TaintAdapter.getMethodKeyForStackFrame(this.methodName, descForStackFrame, false)); + super.visitLdcInsn(getMethodKeyForStackFrame(this.methodName, descForStackFrame, false)); STACK_FRAME_FOR_METHOD_DEBUG.delegateVisit(mv); } else { - push(PhosphorStackFrame.hashForDesc(TaintAdapter.getMethodKeyForStackFrame(this.methodName, descForStackFrame, false))); + push(getHashForStackFrame(this.methodName, descForStackFrame, false)); STACK_FRAME_FOR_METHOD_FAST.delegateVisit(mv); } super.visitInsn(DUP); @@ -784,17 +803,11 @@ protected void prepareMetadataLocalVariables(){ super.visitInsn(POP); super.visitVarInsn(ASTORE, getIndexOfPhosphorStackData()); } else if (Configuration.DEBUG_STACK_FRAME_WRAPPERS) { - String descForStackFrame = this.methodDesc; - if (this.className.startsWith("java/lang/invoke/VarHandleReferences")) { - if (this.methodDesc.contains("java/lang/invoke/VarHandle")) { - descForStackFrame = this.methodDesc.replace("Ljava/lang/invoke/VarHandle;", ""); - } - } - descForStackFrame = descForStackFrame.replace(PhosphorStackFrame.DESCRIPTOR, ""); + String descForStackFrame = fixDescForStack(className, methodDesc) + .replace(PhosphorStackFrame.DESCRIPTOR, ""); super.visitVarInsn(ALOAD, getIndexOfPhosphorStackData()); - super.visitLdcInsn(TaintAdapter.getMethodKeyForStackFrame(this.methodName, descForStackFrame, false)); + super.visitLdcInsn(getMethodKeyForStackFrame(this.methodName, descForStackFrame, false)); CHECK_STACK_FRAME_TARGET.delegateVisit(mv); - } for (int i = this.lvs.getLocalVariableAdder().getIndexOfFirstStackTaintTag(); i < this.lvs.getLocalVariableAdder().getIndexOfLastStackTaintTag(); i++) { TaintMethodRecord.NEW_EMPTY_TAINT.delegateVisit(mv); diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintMethodRecord.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintMethodRecord.java index 8dc2a78cc..aa40cf196 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintMethodRecord.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintMethodRecord.java @@ -62,6 +62,7 @@ public enum TaintMethodRecord implements MethodRecord { START_STACK_FRAME_TRACKING(INVOKESTATIC, PhosphorStackFrame.class, "initialize", Void.TYPE, false), PREPARE_FOR_CALL_DEBUG(INVOKEVIRTUAL, PhosphorStackFrame.class, "prepareForCall", Void.TYPE, false, String.class), PREPARE_FOR_CALL_FAST(INVOKEVIRTUAL, PhosphorStackFrame.class, "prepareForCall", Void.TYPE, false, int.class), + PREPARE_FOR_CALL_PATCHED(INVOKEVIRTUAL, PhosphorStackFrame.class, "prepareForCallPatched", Void.TYPE, false, int.class), PREPARE_FOR_CALL_PREV(INVOKEVIRTUAL, PhosphorStackFrame.class, "prepareForCallPrev", Void.TYPE, false), CHECK_STACK_FRAME_TARGET(INVOKEVIRTUAL, PhosphorStackFrame.class, "checkTarget", Void.TYPE, false, String.class), STACK_FRAME_FOR_METHOD_DEBUG(INVOKESTATIC, PhosphorStackFrame.class, "forMethod", PhosphorStackFrame.class, false, String.class), diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintPassingMV.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintPassingMV.java index 87bc5a56b..bf3e7d386 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintPassingMV.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintPassingMV.java @@ -50,6 +50,7 @@ public class TaintPassingMV extends TaintAdapter implements Opcodes { private boolean doNotUnboxTaints; private boolean isAtStartOfExceptionHandler; private boolean isRewrittenMethodDescriptorOrName; + private final boolean isHandleGuard; private int idxOfShouldPopPhosphorStackFrameLV; @@ -71,6 +72,7 @@ public TaintPassingMV(MethodVisitor mv, int access, String owner, String name, S || (owner.equals("java/io/ObjectInputStream") && name.startsWith("defaultReadFields")); this.controlFlowPolicy = controlFlowPolicy; this.isRewrittenMethodDescriptorOrName = false; + this.isHandleGuard = owner.startsWith("java/lang/invoke/VarHandleGuards") && name.startsWith("guard_"); } @Override @@ -86,10 +88,6 @@ public void visitCode() { ensureObjsAreWrapped = true; //TODO should we always do this? just in some places? performance is what? If try everywhere, it crashes... } } - boolean pullWrapperFromPrev = false; - if (this.className.startsWith("java/lang/invoke/VarHandleGuards") && this.methodName.startsWith("guard_")) { - pullWrapperFromPrev = true; - } //Retrieve the taint tags for all arguments Type[] args = Type.getArgumentTypes(descriptor); @@ -136,7 +134,7 @@ public void visitCode() { super.visitVarInsn(ALOAD, idxOfLV); BOX_IF_NECESSARY.delegateVisit(mv); super.visitVarInsn(ASTORE, idxOfLV); - } else if (pullWrapperFromPrev && args[i].getInternalName().equals("java/lang/Object")) { + } else if (isHandleGuard && args[i].getInternalName().equals("java/lang/Object")) { super.visitInsn(DUP); super.visitFieldInsn(GETFIELD, PhosphorStackFrame.INTERNAL_NAME, "prevFrame", PhosphorStackFrame.DESCRIPTOR); push(i - 1); @@ -619,7 +617,7 @@ public void visitInvokeDynamicInsn(String name, String desc, Handle bsm, Object. super.visitLdcInsn(name + desc.substring(0, 1 + desc.indexOf(')'))); PREPARE_FOR_CALL_DEBUG.delegateVisit(mv); } else { - push(PhosphorStackFrame.hashForDesc(name + desc.substring(0, 1 + desc.indexOf(')')))); + push(PhosphorStackFrame.computeFrameHash(name, desc.substring(0, 1 + desc.indexOf(')')))); PREPARE_FOR_CALL_FAST.delegateVisit(mv); } super.visitInvokeDynamicInsn(name, desc, bsm, bsmArgs); @@ -863,6 +861,16 @@ private static boolean isIgnoredMethod(String owner, String name, String desc) { } private String prepareForCallTo(String owner, String name, String desc) { + if (isHandleGuard && owner.equals("java/lang/invoke/MethodHandle") && name.startsWith("linkTo")) { + String frameDesc = TaintUtils.getOriginalDescrptor(this.methodDesc) + .replace("Ljava/lang/invoke/VarHandle;", "") + .replace("Ljava/lang/invoke/VarHandle$AccessDescriptor;", ""); + frameDesc = frameDesc.substring(0, 1 + frameDesc.indexOf(')')); + int hash = PhosphorStackFrame.computeLowerHash(frameDesc); + push(hash); + PREPARE_FOR_CALL_PATCHED.delegateVisit(mv); + return desc; + } if (isIgnoredMethod(owner, name, desc)) { /* Some methods should never get a stack frame prepared - in particular, a call to @@ -870,19 +878,17 @@ private String prepareForCallTo(String owner, String name, String desc) { some prior caller was trying to reach. In particular, this clearly is needed for MethodHandles.linkTo*, which are used by, e.g. VarHandles. */ - boolean usePrevFrameInsteadOfPreparingNew = false; - if (className.startsWith("java/lang/invoke") && owner.equals("java/lang/invoke/MethodHandle") && name.startsWith("linkTo")) { - usePrevFrameInsteadOfPreparingNew = true; - } - if (usePrevFrameInsteadOfPreparingNew) { + if (className.startsWith("java/lang/invoke") && owner.equals("java/lang/invoke/MethodHandle") + && name.startsWith("linkTo")) { PREPARE_FOR_CALL_PREV.delegateVisit(mv); } else { - String methodKey = getMethodKeyForStackFrame(name, desc, Instrumenter.isPolymorphicSignatureMethod(owner, name)); if (Configuration.DEBUG_STACK_FRAME_WRAPPERS) { + String methodKey = getMethodKeyForStackFrame(name, desc, Instrumenter.isPolymorphicSignatureMethod(owner, name)); mv.visitLdcInsn(methodKey); PREPARE_FOR_CALL_DEBUG.delegateVisit(mv); } else { - push(PhosphorStackFrame.hashForDesc(methodKey)); + push(TaintAdapter.getHashForStackFrame(name, desc, + Instrumenter.isPolymorphicSignatureMethod(owner, name))); PREPARE_FOR_CALL_FAST.delegateVisit(mv); } } diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintTrackingClassVisitor.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintTrackingClassVisitor.java index c0037925b..cfe8843ae 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintTrackingClassVisitor.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/TaintTrackingClassVisitor.java @@ -664,20 +664,12 @@ private void generateMethodWrapper(MethodNode mn) { int stackFrameLocal; int shouldPopLocal; if(fetchStackFrame) { - //There are methods that the JDK will internally resolve to, and prefix the arguments - //with a handle to the underlying object. That handle won't be passed explicitly by the caller. - String descForStackFrame = mn.desc; - if (this.className.startsWith("java/lang/invoke/VarHandleReferences")) { - if (mn.desc.contains("java/lang/invoke/VarHandle")) { - descForStackFrame = mn.desc.replace("Ljava/lang/invoke/VarHandle;", ""); - } - } - + String descForStackFrame = TaintAdapter.fixDescForStack(className, mn.desc); if (Configuration.DEBUG_STACK_FRAME_WRAPPERS) { ga.visitLdcInsn(TaintAdapter.getMethodKeyForStackFrame(mn.name, descForStackFrame, false)); STACK_FRAME_FOR_METHOD_DEBUG.delegateVisit(ga); } else { - ga.push(PhosphorStackFrame.hashForDesc(TaintAdapter.getMethodKeyForStackFrame(mn.name, descForStackFrame, false))); + ga.push(TaintAdapter.getHashForStackFrame(mn.name, descForStackFrame, false)); STACK_FRAME_FOR_METHOD_FAST.delegateVisit(ga); } stackFrameLocal = ga.newLocal(Type.getType(PhosphorStackFrame.class)); @@ -686,7 +678,6 @@ private void generateMethodWrapper(MethodNode mn) { GET_AND_CLEAR_CLEANUP_FLAG.delegateVisit(ga); ga.storeLocal(shouldPopLocal); ga.storeLocal(stackFrameLocal); - /* Fix for #188: We should ensure at this point that any java.lang.Object arg is the expected wrapper type. We should *not* do that inside of Unsafe though, because Phosphor relies on the ability of getReference/putReference @@ -746,7 +737,7 @@ to set underlying array fields (getting this wrong can manifest as JVM segfaults ga.visitLdcInsn(TaintAdapter.getMethodKeyForStackFrame(mn.name, descToCall, false)); PREPARE_FOR_CALL_DEBUG.delegateVisit(ga); } else { - ga.push(PhosphorStackFrame.hashForDesc(TaintAdapter.getMethodKeyForStackFrame(mn.name, descToCall, false))); + ga.push(TaintAdapter.getHashForStackFrame(mn.name, descToCall, false)); PREPARE_FOR_CALL_FAST.delegateVisit(ga); } } diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/UninstrumentedCompatMV.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/UninstrumentedCompatMV.java index 1b275e631..36b704c4e 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/UninstrumentedCompatMV.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/instrumenter/UninstrumentedCompatMV.java @@ -338,7 +338,7 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, super.visitLdcInsn(name + desc.substring(0, 1 + desc.indexOf(')'))); PREPARE_FOR_CALL_DEBUG.delegateVisit(mv); } else { - push(PhosphorStackFrame.hashForDesc(name + desc.substring(0, 1 + desc.indexOf(')')))); + push(PhosphorStackFrame.computeFrameHash(name, desc.substring(0, 1 + desc.indexOf(')')))); PREPARE_FOR_CALL_FAST.delegateVisit(mv); } Type[] args = Type.getArgumentTypes(desc); diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/PhosphorStackFrame.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/PhosphorStackFrame.java index 68e215bb8..6d0bd9fed 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/PhosphorStackFrame.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/PhosphorStackFrame.java @@ -33,8 +33,17 @@ public PhosphorStackFrame(PhosphorStackFrame prevFrame) { this.needsCleanup = true; } - public static int hashForDesc(String intendedNextMethodDebug) { - return intendedNextMethodDebug.hashCode(); + public static int computeFrameHash(String name, String desc) { + int upper = name.hashCode(); + return (upper & 0xffff0000) | computeLowerHash(desc); + } + + public static int computeLowerHash(String desc) { + return (desc.hashCode() & 0x0000ffff); + } + + private void patchLowerHash(int lower) { + intendedNextMethodFast = (intendedNextMethodFast & 0xffff0000) | (lower & 0x0000ffff); } private static void setForThread(PhosphorStackFrame forThread) { @@ -99,6 +108,16 @@ public void prepareForCallPrev() { } } + @InvokedViaInstrumentation(record = TaintMethodRecord.PREPARE_FOR_CALL_PATCHED) + public void prepareForCallPatched(int lower) { + if (initialized){ + if (this.prevFrame != null) { + this.prevFrame.patchLowerHash(lower); + } + setForThread(this.prevFrame); + } + } + @InvokedViaInstrumentation(record = TaintMethodRecord.SET_ARG_WRAPPER) public void setArgWrapper(Object val, int idx) { ensureArgsLength(idx); diff --git a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/TaintSourceWrapper.java b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/TaintSourceWrapper.java index d51cd568c..451b51228 100644 --- a/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/TaintSourceWrapper.java +++ b/Phosphor/src/main/java/edu/columbia/cs/psl/phosphor/runtime/TaintSourceWrapper.java @@ -272,7 +272,7 @@ public static TaggedArray getStringValueTag(CharSequence str) { if(Configuration.DEBUG_STACK_FRAME_WRAPPERS) { frame.prepareForCall("charAt(I)"); } else{ - frame.prepareForCall(PhosphorStackFrame.hashForDesc("charAt(I)")); + frame.prepareForCall(PhosphorStackFrame.computeFrameHash("charAt", "(I)")); } char c = str.charAt(i); Taint tag = frame.getReturnTaint(); diff --git a/phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorJLinkPlugin.java b/phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorJLinkPlugin.java index 80951f09a..304885b65 100644 --- a/phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorJLinkPlugin.java +++ b/phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorJLinkPlugin.java @@ -8,12 +8,9 @@ import jdk.tools.jlink.plugin.ResourcePoolEntry; import java.io.File; -import java.io.IOException; -import java.io.InputStream; import java.net.URISyntaxException; import java.util.*; -import java.util.zip.ZipEntry; -import java.util.zip.ZipFile; + public class PhosphorJLinkPlugin implements Plugin { public static final String NAME = "phosphor-transformer"; @@ -30,8 +27,6 @@ public Category getType() { } /** - * @param jvmDir the source directory where the JDK or JRE is installed - * @param instJVMDir the target directory where the Phosphor-instrumented JVM should be created * @param properties canonicalized properties that specify the Phosphor configuration options that should set in the * created arguments * @return an array formatted for {@link Instrumenter#main(String[])} Instrumenter.main's} String[] argument @@ -68,6 +63,7 @@ public void configure(Map config) { public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) { TaintUtils.VERIFY_CLASS_GENERATION = true; PreMain.RUNTIME_INST = false; + PhosphorPacker packer = new PhosphorPacker(in, phosphorJar); in.transformAndCopy((resourcePoolEntry) -> { if (resourcePoolEntry.type().equals(ResourcePoolEntry.Type.CLASS_OR_RESOURCE)) { if (resourcePoolEntry.path().endsWith(".class")) { @@ -75,40 +71,7 @@ public ResourcePool transform(ResourcePool in, ResourcePoolBuilder out) { //if this is the module for java-base, hack it to export phosphor, and pack phosphor into the module if (resourcePoolEntry.path().startsWith("/java.base")) { //This is the java.base module-info.class file. Transform it, and then add in phosphor - try { - ZipFile phosphorZip = new ZipFile(phosphorJar); - Enumeration phosphorEntries = phosphorZip.entries(); - HashSet packages = new HashSet<>(); - while (phosphorEntries.hasMoreElements()) { - ZipEntry pe = phosphorEntries.nextElement(); - if (pe.getName().startsWith("edu/columbia/cs/psl/jigsaw/phosphor/instrumenter") - || pe.getName().endsWith("module-info.class") - || pe.getName().startsWith("org/") - || pe.getName().startsWith("edu/columbia/cs/psl/phosphor/runtime/jdk/unsupported")) { - continue; - } else if (pe.getName().endsWith(".class")) { - InputStream is = phosphorZip.getInputStream(pe); - if(Instrumenter.isPhosphorClassPatchedAtInstTime(pe.getName())){ - byte[] newContent = Instrumenter.patchPhosphorClass(pe.getName(), is); - out.add(ResourcePoolEntry.create("/java.base/" + pe.getName(), newContent)); - } else { - out.add(ResourcePoolEntry.create("/java.base/" + pe.getName(), is.readAllBytes())); - } - is.close(); - - //package name - packages.add(pe.getName().substring(0, pe.getName().lastIndexOf('/'))); - } - } - phosphorZip.close(); - - byte[] newContent = Instrumenter.transformJavaBaseModuleInfo(resourcePoolEntry.content(), packages); - if (newContent != null) { - resourcePoolEntry = resourcePoolEntry.copyWithContent(newContent); - } - } catch (IOException ex) { - ex.printStackTrace(); - } + resourcePoolEntry = packer.pack(resourcePoolEntry, out); } } else { byte[] newContent = Instrumenter.instrumentClass(resourcePoolEntry.path(), resourcePoolEntry.content(), true); @@ -143,8 +106,6 @@ public static File getPhosphorJarFile() { throw new AssertionError(); } } - - } diff --git a/phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorPacker.java b/phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorPacker.java new file mode 100644 index 000000000..1ebf98b29 --- /dev/null +++ b/phosphor-instrument-jigsaw/src/main/java/edu/columbia/cs/psl/jigsaw/phosphor/instrumenter/PhosphorPacker.java @@ -0,0 +1,74 @@ +package edu.columbia.cs.psl.jigsaw.phosphor.instrumenter; + +import edu.columbia.cs.psl.phosphor.PhosphorPatcher; +import jdk.tools.jlink.plugin.ResourcePool; +import jdk.tools.jlink.plugin.ResourcePoolBuilder; +import jdk.tools.jlink.plugin.ResourcePoolEntry; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Set; +import java.util.zip.ZipEntry; +import java.util.zip.ZipFile; + +class PhosphorPacker { + private final File phosphorJar; + private final PhosphorPatcher patcher; + + PhosphorPacker(ResourcePool pool, File phosphorJar) { + if (phosphorJar == null) { + throw new NullPointerException(); + } + this.phosphorJar = phosphorJar; + ResourcePoolEntry entry = pool.findEntry("/java.base/jdk/internal/misc/Unsafe.class") + .orElseThrow(() -> new IllegalArgumentException("Unable to find entry for jdk/internal/misc/Unsafe")); + try (InputStream in = entry.content()) { + this.patcher = new PhosphorPatcher(in); + } catch (IOException e) { + throw new RuntimeException("Unable to read entry for jdk/internal/misc/Unsafe", e); + } + } + + private boolean shouldInclude(String name) { + return !name.startsWith("edu/columbia/cs/psl/jigsaw/phosphor/instrumenter") + && !name.endsWith("module-info.class") + && !name.startsWith("org/") + && !name.startsWith("edu/columbia/cs/psl/phosphor/runtime/jdk/unsupported") + && name.endsWith(".class"); + } + + ResourcePoolEntry pack(ResourcePoolEntry entry, ResourcePoolBuilder out) { + // Transform java.base's module-info.class file and pack Phosphor classes into java.base + try { + Set packages = packClasses(out); + try (InputStream in = entry.content()) { + return entry.copyWithContent(patcher.transformBaseModuleInfo(in, packages)); + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private Set packClasses(ResourcePoolBuilder out) throws IOException { + // Pack the Phosphor JAR into the resource pool + // Return the set of packages for packed classes + Set packages = new HashSet<>(); + try (ZipFile zip = new ZipFile(phosphorJar)) { + Enumeration entries = zip.entries(); + while (entries.hasMoreElements()) { + ZipEntry entry = entries.nextElement(); + if (shouldInclude(entry.getName())) { + try (InputStream is = zip.getInputStream(entry)) { + byte[] content = patcher.patch(entry.getName(), is.readAllBytes()); + out.add(ResourcePoolEntry.create("/java.base/" + entry.getName(), content)); + } + packages.add(entry.getName().substring(0, entry.getName().lastIndexOf('/'))); + } + } + } + return packages; + } +} diff --git a/pom.xml b/pom.xml index f7a1824ab..78315cdd3 100644 --- a/pom.xml +++ b/pom.xml @@ -92,14 +92,6 @@ maven-dependency-plugin 3.1.1 - - org.apache.maven.plugins - maven-compiler-plugin - - 8 - 8 - - org.apache.maven.plugins maven-checkstyle-plugin