Validate dex paths and class loader contexts reported by apps.
Invalid dex paths and class loader contexts are considered as fatal
errors. We should not let apps trigger those errors.
Bug: 295271514
Test: atest CtsCompilationTestCases
Ignore-AOSP-First: ART Service.
Change-Id: I0431da3db024f77623d559eafe3b144ef532d31f
diff --git a/artd/artd.cc b/artd/artd.cc
index 4558b33..502942e 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -1103,6 +1103,44 @@
return NonFatal(ART_FORMAT("Fstab entries not found for '{}'", in_dexFile));
}
+ScopedAStatus Artd::validateDexPath(const std::string& in_dexPath,
+ std::optional<std::string>* _aidl_return) {
+ if (Result<void> result = ValidateDexPath(in_dexPath); !result.ok()) {
+ *_aidl_return = result.error().message();
+ } else {
+ *_aidl_return = std::nullopt;
+ }
+ return ScopedAStatus::ok();
+}
+
+ScopedAStatus Artd::validateClassLoaderContext(const std::string& in_dexPath,
+ const std::string& in_classLoaderContext,
+ std::optional<std::string>* _aidl_return) {
+ if (in_classLoaderContext == ClassLoaderContext::kUnsupportedClassLoaderContextEncoding) {
+ *_aidl_return = std::nullopt;
+ return ScopedAStatus::ok();
+ }
+
+ std::unique_ptr<ClassLoaderContext> context = ClassLoaderContext::Create(in_classLoaderContext);
+ if (context == nullptr) {
+ *_aidl_return = ART_FORMAT("Class loader context '{}' is invalid", in_classLoaderContext);
+ return ScopedAStatus::ok();
+ }
+
+ std::vector<std::string> flattened_context = context->FlattenDexPaths();
+ std::string dex_dir = Dirname(in_dexPath);
+ for (const std::string& context_element : flattened_context) {
+ std::string context_path = std::filesystem::path(dex_dir).append(context_element);
+ if (Result<void> result = ValidateDexPath(context_path); !result.ok()) {
+ *_aidl_return = result.error().message();
+ return ScopedAStatus::ok();
+ }
+ }
+
+ *_aidl_return = std::nullopt;
+ return ScopedAStatus::ok();
+}
+
Result<void> Artd::Start() {
OR_RETURN(SetLogVerbosity());
diff --git a/artd/artd.h b/artd/artd.h
index 2386cfb..a4012c6 100644
--- a/artd/artd.h
+++ b/artd/artd.h
@@ -166,6 +166,13 @@
ndk::ScopedAStatus isInDalvikCache(const std::string& in_dexFile, bool* _aidl_return) override;
+ ndk::ScopedAStatus validateDexPath(const std::string& in_dexPath,
+ std::optional<std::string>* _aidl_return) override;
+
+ ndk::ScopedAStatus validateClassLoaderContext(const std::string& in_dexPath,
+ const std::string& in_classLoaderContext,
+ std::optional<std::string>* _aidl_return) override;
+
android::base::Result<void> Start();
private:
diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl
index 7440348..ec57bd4 100644
--- a/artd/binder/com/android/server/art/IArtd.aidl
+++ b/artd/binder/com/android/server/art/IArtd.aidl
@@ -177,4 +177,17 @@
* Throws fatal and non-fatal errors.
*/
boolean isInDalvikCache(@utf8InCpp String dexFile);
+
+ /**
+ * Returns an error message if the given dex path is invalid, or null if the validation
+ * passes.
+ */
+ @nullable @utf8InCpp String validateDexPath(@utf8InCpp String dexPath);
+
+ /**
+ * Returns an error message if the given class loader context is invalid, or null if the
+ * validation passes.
+ */
+ @nullable @utf8InCpp String validateClassLoaderContext(@utf8InCpp String dexPath,
+ @utf8InCpp String classLoaderContext);
}
diff --git a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java
index 153e83b..89fb542 100644
--- a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java
+++ b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java
@@ -77,6 +77,7 @@
import java.util.UUID;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
+import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -562,7 +563,7 @@
}
mDexUse = new DexUse();
if (proto != null) {
- mDexUse.fromProto(proto);
+ mDexUse.fromProto(proto, this::validateDexPath, this::validateClassLoaderContext);
}
}
}
@@ -587,7 +588,7 @@
return !loader.loadingPackageName().equals(owningPackageName) || loader.isolatedProcess();
}
- private static void validateInputs(@NonNull PackageManagerLocal.FilteredSnapshot snapshot,
+ private void validateInputs(@NonNull PackageManagerLocal.FilteredSnapshot snapshot,
@NonNull String loadingPackageName,
@NonNull Map<String, String> classLoaderContextByDexContainerFile) {
if (classLoaderContextByDexContainerFile.isEmpty()) {
@@ -596,11 +597,15 @@
for (var entry : classLoaderContextByDexContainerFile.entrySet()) {
Utils.assertNonEmpty(entry.getKey());
- if (!Paths.get(entry.getKey()).isAbsolute()) {
- throw new IllegalArgumentException(String.format(
- "Dex container file path must be absolute, got '%s'", entry.getKey()));
+ String errorMsg = validateDexPath(entry.getKey());
+ if (errorMsg != null) {
+ throw new IllegalArgumentException(errorMsg);
}
Utils.assertNonEmpty(entry.getValue());
+ errorMsg = validateClassLoaderContext(entry.getKey(), entry.getValue());
+ if (errorMsg != null) {
+ throw new IllegalArgumentException(errorMsg);
+ }
}
// TODO(b/253570365): Make the validation more strict.
@@ -615,6 +620,29 @@
}
}
+ @Nullable
+ private String validateDexPath(@NonNull String dexPath) {
+ try {
+ return mInjector.getArtd().validateDexPath(dexPath);
+ } catch (RemoteException e) {
+ String errorMsg = "Failed to validate dex path " + dexPath;
+ Log.e(TAG, errorMsg, e);
+ return errorMsg;
+ }
+ }
+
+ @Nullable
+ private String validateClassLoaderContext(
+ @NonNull String dexPath, @NonNull String classLoaderContext) {
+ try {
+ return mInjector.getArtd().validateClassLoaderContext(dexPath, classLoaderContext);
+ } catch (RemoteException e) {
+ String errorMsg = "Failed to validate class loader context " + classLoaderContext;
+ Log.e(TAG, errorMsg, e);
+ return errorMsg;
+ }
+ }
+
/** @hide */
@Nullable
public String getSecondaryClassLoaderContext(
@@ -878,10 +906,12 @@
}
}
- void fromProto(@NonNull DexUseProto proto) {
+ void fromProto(@NonNull DexUseProto proto,
+ @NonNull Function<String, String> validateDexPath,
+ @NonNull BiFunction<String, String, String> validateClassLoaderContext) {
for (PackageDexUseProto packageProto : proto.getPackageDexUseList()) {
var packageDexUse = new PackageDexUse();
- packageDexUse.fromProto(packageProto);
+ packageDexUse.fromProto(packageProto, validateDexPath, validateClassLoaderContext);
mPackageDexUseByOwningPackageName.put(
Utils.assertNonEmpty(packageProto.getOwningPackageName()), packageDexUse);
}
@@ -914,7 +944,9 @@
}
}
- void fromProto(@NonNull PackageDexUseProto proto) {
+ void fromProto(@NonNull PackageDexUseProto proto,
+ @NonNull Function<String, String> validateDexPath,
+ @NonNull BiFunction<String, String, String> validateClassLoaderContext) {
for (PrimaryDexUseProto primaryProto : proto.getPrimaryDexUseList()) {
var primaryDexUse = new PrimaryDexUse();
primaryDexUse.fromProto(primaryProto);
@@ -922,10 +954,20 @@
Utils.assertNonEmpty(primaryProto.getDexFile()), primaryDexUse);
}
for (SecondaryDexUseProto secondaryProto : proto.getSecondaryDexUseList()) {
+ String dexFile = Utils.assertNonEmpty(secondaryProto.getDexFile());
+
+ // Skip invalid dex paths persisted by previous versions.
+ String errorMsg = validateDexPath.apply(dexFile);
+ if (errorMsg != null) {
+ Log.e(TAG, errorMsg);
+ continue;
+ }
+
var secondaryDexUse = new SecondaryDexUse();
- secondaryDexUse.fromProto(secondaryProto);
- mSecondaryDexUseByDexFile.put(
- Utils.assertNonEmpty(secondaryProto.getDexFile()), secondaryDexUse);
+ secondaryDexUse.fromProto(secondaryProto,
+ classLoaderContext
+ -> validateClassLoaderContext.apply(dexFile, classLoaderContext));
+ mSecondaryDexUseByDexFile.put(dexFile, secondaryDexUse);
}
}
}
@@ -972,10 +1014,19 @@
}
}
- void fromProto(@NonNull SecondaryDexUseProto proto) {
+ void fromProto(@NonNull SecondaryDexUseProto proto,
+ @NonNull Function<String, String> validateClassLoaderContext) {
Utils.check(proto.hasUserId());
mUserHandle = UserHandle.of(proto.getUserId().getValue());
for (SecondaryDexUseRecordProto recordProto : proto.getRecordList()) {
+ // Skip invalid class loader context persisted by previous versions.
+ String errorMsg = validateClassLoaderContext.apply(
+ Utils.assertNonEmpty(recordProto.getClassLoaderContext()));
+ if (errorMsg != null) {
+ Log.e(TAG, errorMsg);
+ continue;
+ }
+
var record = new SecondaryDexUseRecord();
record.fromProto(recordProto);
mRecordByLoader.put(
diff --git a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java
index 5850e61..cf27d8c 100644
--- a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java
+++ b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java
@@ -147,6 +147,9 @@
mTempFile = File.createTempFile("package-dex-usage", ".pb");
mTempFile.deleteOnExit();
+ lenient().when(mArtd.validateDexPath(any())).thenReturn(null);
+ lenient().when(mArtd.validateClassLoaderContext(any(), any())).thenReturn(null);
+
lenient().when(mInjector.getArtd()).thenReturn(mArtd);
lenient().when(mInjector.getCurrentTimeMillis()).thenReturn(0l);
lenient().when(mInjector.getFilename()).thenReturn(mTempFile.getPath());
@@ -699,9 +702,17 @@
}
@Test(expected = IllegalArgumentException.class)
- public void testNonAbsoluteKey() {
+ public void testInvalidDexPath() throws Exception {
+ lenient().when(mArtd.validateDexPath(any())).thenReturn("invalid");
mDexUseManager.notifyDexContainersLoaded(
- mSnapshot, OWNING_PKG_NAME, Map.of("a/b.jar", "CLC"));
+ mSnapshot, OWNING_PKG_NAME, Map.of("/a/b.jar", "PCL[]"));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testInvalidClassLoaderContext() throws Exception {
+ lenient().when(mArtd.validateClassLoaderContext(any(), any())).thenReturn("invalid");
+ mDexUseManager.notifyDexContainersLoaded(
+ mSnapshot, OWNING_PKG_NAME, Map.of("/a/b.jar", "PCL[]"));
}
@Test(expected = IllegalArgumentException.class)
