Merge "Add descriptor validation to ClassLinker::FindClass()."

7 files changed, 433 insertions, 1 deletions

diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 4bc8e8efba..1d0684202c 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -2592,9 +2592,25 @@ mirror::Class* ClassLinker::FindClass(Thread* self,
         return nullptr;
       }
 
+      // Inlined DescriptorToDot(descriptor) with extra validation.
+      //
+      // Throw NoClassDefFoundError early rather than potentially load a class only to fail
+      // the DescriptorEquals() check below and give a confusing error message. For example,
+      // when native code erroneously calls JNI GetFieldId() with signature "java/lang/String"
+      // instead of "Ljava/lang/String;", the message below using the "dot" names would be
+      // "class loader [...] returned class java.lang.String instead of java.lang.String".
+      size_t descriptor_length = strlen(descriptor);
+      if (UNLIKELY(descriptor[0] != 'L') ||
+          UNLIKELY(descriptor[descriptor_length - 1] != ';') ||
+          UNLIKELY(memchr(descriptor + 1, '.', descriptor_length - 2) != nullptr)) {
+        ThrowNoClassDefFoundError("Invalid descriptor: %s.", descriptor);
+        return nullptr;
+      }
+      std::string class_name_string(descriptor + 1, descriptor_length - 2);
+      std::replace(class_name_string.begin(), class_name_string.end(), '/', '.');
+
       ScopedLocalRef<jobject> class_loader_object(
           soa.Env(), soa.AddLocalReference<jobject>(class_loader.Get()));
-      std::string class_name_string(DescriptorToDot(descriptor));
       ScopedLocalRef<jobject> result(soa.Env(), nullptr);
       {
         ScopedThreadStateChange tsc(self, kNative);
diff --git a/test/647-jni-get-field-id/expected.txt b/test/647-jni-get-field-id/expected.txt
new file mode 100644
index 0000000000..9506dd7806
--- /dev/null
+++ b/test/647-jni-get-field-id/expected.txt
@@ -0,0 +1,26 @@
+JNI_OnLoad called
+getFieldId(class TestClass, "intField", "I")
+Result: true
+getFieldId(class TestClass, "intField", "int")
+Caught java.lang.NoSuchFieldError
+  caused by java.lang.NoClassDefFoundError
+getFieldId(class TestClass, "intField", "Lint;")
+Caught java.lang.NoSuchFieldError
+  caused by java.lang.ClassNotFoundException
+getFieldId(class TestClass, "stringField", "I")
+Caught java.lang.NoSuchFieldError
+getFieldId(class TestClass, "stringField", "Ljava/lang/String;")
+Result: true
+getFieldId(class TestClass, "stringField", "java/lang/String")
+Caught java.lang.NoSuchFieldError
+  caused by java.lang.NoClassDefFoundError
+getFieldId(class TestClass, "stringField", "Ljava.lang.String;")
+Caught java.lang.NoSuchFieldError
+  caused by java.lang.NoClassDefFoundError
+getFieldId(class TestClass, "stringField", "java.lang.String")
+Caught java.lang.NoSuchFieldError
+  caused by java.lang.NoClassDefFoundError
+Test that MyClassLoader.loadClass("Bad.Class") shall not be called.
+  Error message for Bad/Class: Invalid descriptor: Bad/Class.
+  Error message for Bad.Class: Invalid descriptor: Bad.Class.
+  Error message for LBad.Class;: Invalid descriptor: LBad.Class;.
diff --git a/test/647-jni-get-field-id/get_field_id.cc b/test/647-jni-get-field-id/get_field_id.cc
new file mode 100644
index 0000000000..2056cfb549
--- /dev/null
+++ b/test/647-jni-get-field-id/get_field_id.cc
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "jni.h"
+
+#include "ScopedUtfChars.h"
+
+namespace art {
+
+extern "C" JNIEXPORT jboolean JNICALL Java_Main_getFieldId(JNIEnv* env,
+                                                           jclass,
+                                                           jclass cls,
+                                                           jstring name,
+                                                           jstring signature) {
+  ScopedUtfChars name_chars(env, name);
+  if (name_chars.c_str() == nullptr) {
+    return false;
+  }
+  ScopedUtfChars signature_chars(env, signature);
+  if (signature_chars.c_str() == nullptr) {
+    return false;
+  }
+  jfieldID field_id = env->GetFieldID(cls, name_chars.c_str(), signature_chars.c_str());
+  if (field_id == nullptr) {
+    return false;
+  }
+  return true;
+}
+
+}  // namespace art
diff --git a/test/647-jni-get-field-id/info.txt b/test/647-jni-get-field-id/info.txt
new file mode 100644
index 0000000000..00a2b20b47
--- /dev/null
+++ b/test/647-jni-get-field-id/info.txt
@@ -0,0 +1 @@
+Test for native calls to JNI GetFieldID() with odd signatures.
diff --git a/test/647-jni-get-field-id/src/DefiningLoader.java b/test/647-jni-get-field-id/src/DefiningLoader.java
new file mode 100644
index 0000000000..8597c11ab9
--- /dev/null
+++ b/test/647-jni-get-field-id/src/DefiningLoader.java
@@ -0,0 +1,239 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.File;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
+import java.lang.reflect.InvocationTargetException;
+
+/**
+ * A class loader with atypical behavior: we try to load a private
+ * class implementation before asking the system or boot loader.  This
+ * is used to create multiple classes with identical names in a single VM.
+ *
+ * If DexFile is available, we use that; if not, we assume we're not in
+ * Dalvik and instantiate the class with defineClass().
+ *
+ * The location of the DEX files and class data is dependent upon the
+ * test framework.
+ */
+public class DefiningLoader extends ClassLoader {
+    static {
+        // For JVM, register as parallel capable.
+        // Android treats all class loaders as parallel capable and makes this a no-op.
+        registerAsParallelCapable();
+    }
+
+    /* this is where the .class files live */
+    static final String CLASS_PATH1 = "classes/";
+    static final String CLASS_PATH2 = "classes2/";
+
+    /* this is the DEX/Jar file */
+    static final String DEX_FILE = System.getenv("DEX_LOCATION") + "/647-jni-get-field-id.jar";
+
+    /* on Dalvik, this is a DexFile; otherwise, it's null */
+    private Class<?> mDexClass;
+
+    private Object mDexFile;
+
+    /**
+     * Construct DefiningLoader, grabbing a reference to the DexFile class
+     * if we're running under Dalvik.
+     */
+    public DefiningLoader(ClassLoader parent) {
+        super(parent);
+
+        try {
+            mDexClass = parent.loadClass("dalvik.system.DexFile");
+        } catch (ClassNotFoundException cnfe) {
+            // ignore -- not running Dalvik
+        }
+    }
+
+    /**
+     * Finds the class with the specified binary name.
+     *
+     * We search for a file in CLASS_PATH or pull an entry from DEX_FILE.
+     * If we don't find a match, we throw an exception.
+     */
+    protected Class<?> findClass(String name) throws ClassNotFoundException
+    {
+        if (mDexClass != null) {
+            return findClassDalvik(name);
+        } else {
+            return findClassNonDalvik(name);
+        }
+    }
+
+    /**
+     * Finds the class with the specified binary name, from a DEX file.
+     */
+    private Class<?> findClassDalvik(String name)
+        throws ClassNotFoundException {
+
+        if (mDexFile == null) {
+            synchronized (DefiningLoader.class) {
+                Constructor<?> ctor;
+                /*
+                 * Construct a DexFile object through reflection.
+                 */
+                try {
+                    ctor = mDexClass.getConstructor(String.class);
+                } catch (NoSuchMethodException nsme) {
+                    throw new ClassNotFoundException("getConstructor failed",
+                        nsme);
+                }
+
+                try {
+                    mDexFile = ctor.newInstance(DEX_FILE);
+                } catch (InstantiationException ie) {
+                    throw new ClassNotFoundException("newInstance failed", ie);
+                } catch (IllegalAccessException iae) {
+                    throw new ClassNotFoundException("newInstance failed", iae);
+                } catch (InvocationTargetException ite) {
+                    throw new ClassNotFoundException("newInstance failed", ite);
+                }
+            }
+        }
+
+        /*
+         * Call DexFile.loadClass(String, ClassLoader).
+         */
+        Method meth;
+
+        try {
+            meth = mDexClass.getMethod("loadClass", String.class, ClassLoader.class);
+        } catch (NoSuchMethodException nsme) {
+            throw new ClassNotFoundException("getMethod failed", nsme);
+        }
+
+        try {
+            meth.invoke(mDexFile, name, this);
+        } catch (IllegalAccessException iae) {
+            throw new ClassNotFoundException("loadClass failed", iae);
+        } catch (InvocationTargetException ite) {
+            throw new ClassNotFoundException("loadClass failed",
+                ite.getCause());
+        }
+
+        return null;
+    }
+
+    /**
+     * Finds the class with the specified binary name, from .class files.
+     */
+    private Class<?> findClassNonDalvik(String name)
+        throws ClassNotFoundException {
+
+        String[] pathNames = { CLASS_PATH1 + name + ".class", CLASS_PATH2 + name + ".class" };
+
+        String pathName = null;
+        RandomAccessFile raf = null;
+
+        for (String pn : pathNames) {
+            pathName = pn;
+            try {
+                //System.out.println("--- Defining: looking for " + pathName);
+                raf = new RandomAccessFile(new File(pathName), "r");
+                break;
+            } catch (FileNotFoundException fnfe) {
+            }
+        }
+        if (raf == null) {
+            throw new ClassNotFoundException("Not found: " + pathNames[0] + ":" + pathNames[1]);
+        }
+
+        /* read the entire file in */
+        byte[] fileData;
+        try {
+            fileData = new byte[(int) raf.length()];
+            raf.readFully(fileData);
+        } catch (IOException ioe) {
+            throw new ClassNotFoundException("Read error: " + pathName);
+        } finally {
+            try {
+                raf.close();
+            } catch (IOException ioe) {
+                // drop
+            }
+        }
+
+        /* create the class */
+        //System.out.println("--- Defining: defining " + name);
+        try {
+            return defineClass(name, fileData, 0, fileData.length);
+        } catch (Throwable th) {
+            throw new ClassNotFoundException("defineClass failed", th);
+        }
+    }
+
+    /**
+     * Load a class.
+     *
+     * Normally a class loader wouldn't override this, but we want our
+     * version of the class to take precedence over an already-loaded
+     * version.
+     *
+     * We still want the system classes (e.g. java.lang.Object) from the
+     * bootstrap class loader.
+     */
+    synchronized protected Class<?> loadClass(String name, boolean resolve)
+        throws ClassNotFoundException
+    {
+        Class<?> res;
+
+        /*
+         * 1. Invoke findLoadedClass(String) to check if the class has
+         * already been loaded.
+         *
+         * This doesn't change.
+         */
+        res = findLoadedClass(name);
+        if (res != null) {
+            // System.out.println("FancyLoader.loadClass: " + name + " already loaded");
+            if (resolve)
+                resolveClass(res);
+            return res;
+        }
+
+        /*
+         * 3. Invoke the findClass(String) method to find the class.
+         */
+        try {
+            res = findClass(name);
+            if (resolve)
+                resolveClass(res);
+        }
+        catch (ClassNotFoundException e) {
+            // we couldn't find it, so eat the exception and keep going
+        }
+
+        /*
+         * 2. Invoke the loadClass method on the parent class loader.  If
+         * the parent loader is null the class loader built-in to the
+         * virtual machine is used, instead.
+         *
+         * (Since we're not in java.lang, we can't actually invoke the
+         * parent's loadClass() method, but we passed our parent to the
+         * super-class which can take care of it for us.)
+         */
+        res = super.loadClass(name, resolve);   // returns class or throws
+        return res;
+    }
+}
diff --git a/test/647-jni-get-field-id/src/Main.java b/test/647-jni-get-field-id/src/Main.java
new file mode 100644
index 0000000000..590ee8a402
--- /dev/null
+++ b/test/647-jni-get-field-id/src/Main.java
@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+public class Main {
+    public static void main(String[] args) {
+        System.loadLibrary(args[0]);
+
+        testGetFieldId(TestClass.class, "intField", "I");
+        testGetFieldId(TestClass.class, "intField", "int");
+        testGetFieldId(TestClass.class, "intField", "Lint;");
+        testGetFieldId(TestClass.class, "stringField", "I");
+        testGetFieldId(TestClass.class, "stringField", "Ljava/lang/String;");
+        testGetFieldId(TestClass.class, "stringField", "java/lang/String");
+        testGetFieldId(TestClass.class, "stringField", "Ljava.lang.String;");
+        testGetFieldId(TestClass.class, "stringField", "java.lang.String");
+
+        try {
+            Method get = Main.class.getDeclaredMethod("getFieldId",
+                                                      Class.class,
+                                                      String.class,
+                                                      String.class);
+            MyClassLoader loader = new MyClassLoader(Main.class.getClassLoader());
+            Class<?> otherMain = Class.forName("Main", true, loader);
+            Method m = otherMain.getDeclaredMethod("testClassLoading", Method.class);
+            m.invoke(null, get);
+        } catch (Throwable t) {
+            t.printStackTrace(System.out);
+        }
+    }
+
+    public static void testClassLoading(Method get) throws Exception {
+        System.out.println("Test that MyClassLoader.loadClass(\"Bad.Class\") shall not be called.");
+        String[] bad_class_names = { "Bad/Class", "Bad.Class", "LBad.Class;" };
+        for (String signature : bad_class_names) {
+            try {
+                get.invoke(null, TestClass.class, "bogus", signature);
+                System.out.println("FAIL!");
+            } catch (InvocationTargetException ite) {
+                if (!(ite.getCause() instanceof NoSuchFieldError) ||
+                    !(ite.getCause().getCause() instanceof NoClassDefFoundError)) {
+                  throw ite;
+                }
+                NoClassDefFoundError ncdfe = (NoClassDefFoundError) ite.getCause().getCause();
+                System.out.println("  Error message for " + signature + ": " + ncdfe.getMessage());
+            }
+        }
+    }
+
+    public static void testGetFieldId(Class<?> cls, String name, String signature) {
+        System.out.println("getFieldId(" + cls + ", \"" + name + "\", \"" + signature + "\")");
+        try {
+            boolean result = getFieldId(cls, name, signature);
+            System.out.println("Result: " + result);
+        } catch (Throwable t) {
+            System.out.println("Caught " + DescribeThrowable(t));
+            for (Throwable cause = t.getCause(); cause != null; cause = cause.getCause()) {
+                System.out.println("  caused by " + DescribeThrowable(cause));
+            }
+        }
+    }
+
+    public static String DescribeThrowable(Throwable t) {
+        return PRINT_MESSAGE ? t.getClass().getName() + ": " + t.getMessage()
+                             : t.getClass().getName();
+    }
+
+    public static native boolean getFieldId(Class<?> cls, String name, String signature);
+
+    // Set to true to see actual messages.
+    public static final boolean PRINT_MESSAGE = false;
+}
+
+class TestClass {
+    public int intField;
+    public String stringField;
+}
+
+class MyClassLoader extends DefiningLoader {
+  public MyClassLoader(ClassLoader parent) {
+      super(parent);
+  }
+
+  public Class<?> loadClass(String name) throws ClassNotFoundException
+  {
+      if (name.equals("Bad.Class")) {
+          throw new Error("findClass(\"Bad.Class\")");
+      }
+      return super.loadClass(name);
+  }
+}
diff --git a/test/Android.bp b/test/Android.bp
index d8a3f0eeed..93fd607c24 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -391,6 +391,7 @@ cc_defaults {
         "597-deopt-new-string/deopt.cc",
         "626-const-class-linking/clear_dex_cache_types.cc",
         "642-fp-callees/fp_callees.cc",
+        "647-jni-get-field-id/get_field_id.cc",
     ],
     shared_libs: [
         "libbacktrace",