From 46bbaa3be86e01aca8d6f34c89cc90a6333fcf96 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Wed, 11 Jun 2025 14:48:25 +0100 Subject: [SP 2025-09-01] Throw an exception in JNI::NewObject for abstract classes. Test: 863-serialization Bug: 421834866 Flag: EXEMPT bugfix (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a4826745b63bdab1db7536680e1c8e947a56f7be) Merged-In: I4ccf22f85b4ae0325e9f8e29503149bbda533e86 Change-Id: I4ccf22f85b4ae0325e9f8e29503149bbda533e86 --- runtime/jni/check_jni.cc | 10 ++--- runtime/jni/jni_internal.cc | 21 ++++++++++ test/863-serialization/expected-stderr.txt | 0 test/863-serialization/expected-stdout.txt | 0 test/863-serialization/info.txt | 2 + test/863-serialization/src/Main.java | 62 ++++++++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 test/863-serialization/expected-stderr.txt create mode 100644 test/863-serialization/expected-stdout.txt create mode 100644 test/863-serialization/info.txt create mode 100644 test/863-serialization/src/Main.java diff --git a/runtime/jni/check_jni.cc b/runtime/jni/check_jni.cc index a05a3e97f2..24e3256d81 100644 --- a/runtime/jni/check_jni.cc +++ b/runtime/jni/check_jni.cc @@ -747,10 +747,10 @@ class ScopedCheck { return true; } - bool CheckInstantiableNonArray(ScopedObjectAccess& soa, jclass jc) + bool CheckNonArray(ScopedObjectAccess& soa, jclass jc) REQUIRES_SHARED(Locks::mutator_lock_) { ObjPtr c = soa.Decode(jc); - if (!c->IsInstantiableNonArray()) { + if (c->IsArrayClass()) { AbortF("can't make objects of type %s: %p", c->PrettyDescriptor().c_str(), c.Ptr()); return false; } @@ -2195,7 +2195,7 @@ class CheckJNI { ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.c = c}}; - if (sc.Check(soa, true, "Ec", args) && sc.CheckInstantiableNonArray(soa, c)) { + if (sc.Check(soa, true, "Ec", args) && sc.CheckNonArray(soa, c)) { JniValueType result; result.L = baseEnv(env)->AllocObject(env, c); if (sc.Check(soa, false, "L", &result)) { @@ -2211,7 +2211,7 @@ class CheckJNI { ScopedCheck sc(kFlag_Default, __FUNCTION__); VarArgs rest(mid, vargs); JniValueType args[4] = {{.E = env}, {.c = c}, {.m = mid}, {.va = &rest}}; - if (sc.Check(soa, true, "Ecm.", args) && sc.CheckInstantiableNonArray(soa, c) && + if (sc.Check(soa, true, "Ecm.", args) && sc.CheckNonArray(soa, c) && sc.CheckConstructor(mid)) { JniValueType result; result.L = baseEnv(env)->NewObjectV(env, c, mid, vargs); @@ -2237,7 +2237,7 @@ class CheckJNI { ScopedCheck sc(kFlag_Default, __FUNCTION__); VarArgs rest(mid, vargs); JniValueType args[4] = {{.E = env}, {.c = c}, {.m = mid}, {.va = &rest}}; - if (sc.Check(soa, true, "Ecm.", args) && sc.CheckInstantiableNonArray(soa, c) && + if (sc.Check(soa, true, "Ecm.", args) && sc.CheckNonArray(soa, c) && sc.CheckConstructor(mid)) { JniValueType result; result.L = baseEnv(env)->NewObjectA(env, c, mid, vargs); diff --git a/runtime/jni/jni_internal.cc b/runtime/jni/jni_internal.cc index 1dde2de741..9129d9ed41 100644 --- a/runtime/jni/jni_internal.cc +++ b/runtime/jni/jni_internal.cc @@ -922,6 +922,13 @@ class JNI { if (c == nullptr) { return nullptr; } + if (UNLIKELY(!c->IsInstantiable())) { + soa.Self()->ThrowNewExceptionF( + "Ljava/lang/InstantiationException;", "Can't instantiate %s %s", + c->IsInterface() ? "interface" : "abstract class", + c->PrettyDescriptor().c_str()); + return nullptr; + } if (c->IsStringClass()) { gc::AllocatorType allocator_type = Runtime::Current()->GetHeap()->GetCurrentAllocator(); return soa.AddLocalReference( @@ -949,6 +956,13 @@ class JNI { if (c == nullptr) { return nullptr; } + if (UNLIKELY(!c->IsInstantiable())) { + soa.Self()->ThrowNewExceptionF( + "Ljava/lang/InstantiationException;", "Can't instantiate %s %s", + c->IsInterface() ? "interface" : "abstract class", + c->PrettyDescriptor().c_str()); + return nullptr; + } if (c->IsStringClass()) { // Replace calls to String. with equivalent StringFactory call. jmethodID sf_mid = jni::EncodeArtMethod( @@ -975,6 +989,13 @@ class JNI { if (c == nullptr) { return nullptr; } + if (UNLIKELY(!c->IsInstantiable())) { + soa.Self()->ThrowNewExceptionF( + "Ljava/lang/InstantiationException;", "Can't instantiate %s %s", + c->IsInterface() ? "interface" : "abstract class", + c->PrettyDescriptor().c_str()); + return nullptr; + } if (c->IsStringClass()) { // Replace calls to String. with equivalent StringFactory call. jmethodID sf_mid = jni::EncodeArtMethod( diff --git a/test/863-serialization/expected-stderr.txt b/test/863-serialization/expected-stderr.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/863-serialization/expected-stdout.txt b/test/863-serialization/expected-stdout.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/863-serialization/info.txt b/test/863-serialization/info.txt new file mode 100644 index 0000000000..a9693a9f64 --- /dev/null +++ b/test/863-serialization/info.txt @@ -0,0 +1,2 @@ +Regression test for JNI::NewObject where we forgot to check if a class is +instantiable. diff --git a/test/863-serialization/src/Main.java b/test/863-serialization/src/Main.java new file mode 100644 index 0000000000..72cc01f896 --- /dev/null +++ b/test/863-serialization/src/Main.java @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2025 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.ByteArrayInputStream; +import java.io.InvalidClassException; +import java.io.ObjectInputStream; + +public class Main { + + public static void main(String[] args) throws Exception { + deserializeHexToConcurrentHashMap(); + } + + public static byte[] hexStringToByteArray(String hexString) { + if (hexString == null || hexString.isEmpty()) { + return new byte[0]; + } + if (hexString.length() % 2 != 0) { + throw new IllegalArgumentException("Hex string must have an even number of characters."); + } + int len = hexString.length(); + byte[] data = new byte[len / 2]; + for (int i = 0; i < len; i += 2) { + int highNibble = Character.digit(hexString.charAt(i), 16); + int lowNibble = Character.digit(hexString.charAt(i + 1), 16); + if (highNibble == -1 || lowNibble == -1) { + throw new IllegalArgumentException( + "Invalid hex character in string: " + hexString.charAt(i) + hexString.charAt(i + 1)); + } + data[i / 2] = (byte) ((highNibble << 4) + lowNibble); + } + return data; + } + + public static void deserializeHexToConcurrentHashMap() throws Exception { + byte[] bytes = hexStringToByteArray("ACED0005737200266A6176612E7574696C2E636F6E63757272656E742E436F6E63757272656E74486173684D61706499DE129D87293D0300007870737200146A6176612E746578742E44617465466F726D6174642CA1E4C22615FC0200007870737200146A6176612E746578742E44617465466F726D6174642CA1E4C22615FC020000787070707878000000"); + ByteArrayInputStream bis = new ByteArrayInputStream(bytes); + ObjectInputStream ois = new ObjectInputStream(bis); + try { + Object deserializedObject = ois.readObject(); + throw new Error("Expected InvalidClassException"); + } catch (InvalidClassException e) { + // expected + if (!(e.getCause() instanceof InstantiationException)) { + throw new Error("Expected InstantiationException"); + } + } + } +} -- cgit v1.2.3-59-g8ed1b