Move initZipPathValidator() into ActivityThread
initZipPathValidator() relies on CompatChanges#isChangeEnabled() to work
as intended, however, the current place, RuntimeInit#commonInit(), has
a default no-op implementation of the CompatChanges callback. This
change moves initZipPathValidator() into
ActivityThread#handleBindApplication right after the CompatChanges
callback is set, meaning the compat check starts working as intended.
This change also removes the unit tests and converts them into CTS.
Bug: 268138388
Bug: 268137846
Test: atest CtsSecurityHostTestCases:android.security.cts.host.ZipPathValidatorTest
Change-Id: I4b7ccfd0f3b48844abe3edc112217478ceb1b300
Merged-In: I539465d818e78a7f1dd4dddbe345331836e09caf
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 1310b83..c929df9 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -36,6 +36,7 @@
import static android.window.ConfigurationHelper.shouldUpdateResources;
import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE;
+import static com.android.internal.os.SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -48,6 +49,7 @@
import android.app.assist.AssistContent;
import android.app.assist.AssistStructure;
import android.app.backup.BackupAgent;
+import android.app.compat.CompatChanges;
import android.app.servertransaction.ActivityLifecycleItem;
import android.app.servertransaction.ActivityLifecycleItem.LifecycleState;
import android.app.servertransaction.ActivityRelaunchItem;
@@ -202,6 +204,7 @@
import com.android.internal.os.BinderCallsStats;
import com.android.internal.os.BinderInternal;
import com.android.internal.os.RuntimeInit;
+import com.android.internal.os.SafeZipPathValidatorCallback;
import com.android.internal.os.SomeArgs;
import com.android.internal.policy.DecorView;
import com.android.internal.util.ArrayUtils;
@@ -216,6 +219,7 @@
import dalvik.system.CloseGuard;
import dalvik.system.VMDebug;
import dalvik.system.VMRuntime;
+import dalvik.system.ZipPathValidator;
import libcore.io.ForwardingOs;
import libcore.io.IoUtils;
@@ -6466,6 +6470,11 @@
// Let libcore handle any compat changes after installing the list of compat changes.
AppSpecializationHooks.handleCompatChangesBeforeBindingApplication();
+ // Initialize the zip path validator callback depending on the targetSdk.
+ // This has to be after AppCompatCallbacks#install() so that the Compat
+ // checks work accordingly.
+ initZipPathValidatorCallback();
+
mBoundApplication = data;
mConfigurationController.setConfiguration(data.config);
mConfigurationController.setCompatConfiguration(data.config);
@@ -6795,6 +6804,19 @@
}
}
+ /**
+ * If targetSDK >= U: set the safe zip path validator callback which disallows dangerous zip
+ * entry names.
+ * Otherwise: clear the callback to the default validation.
+ */
+ private void initZipPathValidatorCallback() {
+ if (CompatChanges.isChangeEnabled(VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL)) {
+ ZipPathValidator.setCallback(new SafeZipPathValidatorCallback());
+ } else {
+ ZipPathValidator.clearCallback();
+ }
+ }
+
private void handleSetContentCaptureOptionsCallback(String packageName) {
if (mContentCaptureOptionsCallback != null) {
return;
diff --git a/core/java/com/android/internal/os/RuntimeInit.java b/core/java/com/android/internal/os/RuntimeInit.java
index 8a9445d..28b98d6 100644
--- a/core/java/com/android/internal/os/RuntimeInit.java
+++ b/core/java/com/android/internal/os/RuntimeInit.java
@@ -16,14 +16,10 @@
package com.android.internal.os;
-import static com.android.internal.os.SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL;
-
-import android.annotation.TestApi;
import android.app.ActivityManager;
import android.app.ActivityThread;
import android.app.ApplicationErrorReport;
import android.app.IActivityManager;
-import android.app.compat.CompatChanges;
import android.compat.annotation.UnsupportedAppUsage;
import android.content.type.DefaultMimeMapFactory;
import android.net.TrafficStats;
@@ -40,7 +36,6 @@
import dalvik.system.RuntimeHooks;
import dalvik.system.VMRuntime;
-import dalvik.system.ZipPathValidator;
import libcore.content.type.MimeMap;
@@ -265,31 +260,10 @@
*/
TrafficStats.attachSocketTagger();
- /*
- * Initialize the zip path validator callback depending on the targetSdk.
- */
- initZipPathValidatorCallback();
-
initialized = true;
}
/**
- * If targetSDK >= U: set the safe zip path validator callback which disallows dangerous zip
- * entry names.
- * Otherwise: clear the callback to the default validation.
- *
- * @hide
- */
- @TestApi
- public static void initZipPathValidatorCallback() {
- if (CompatChanges.isChangeEnabled(VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL)) {
- ZipPathValidator.setCallback(new SafeZipPathValidatorCallback());
- } else {
- ZipPathValidator.clearCallback();
- }
- }
-
- /**
* Returns an HTTP user agent of the form
* "Dalvik/1.1.0 (Linux; U; Android Eclair Build/MAIN)".
*/
diff --git a/core/tests/coretests/src/com/android/internal/os/SafeZipPathValidatorCallbackTest.java b/core/tests/coretests/src/com/android/internal/os/SafeZipPathValidatorCallbackTest.java
deleted file mode 100644
index c540a15..0000000
--- a/core/tests/coretests/src/com/android/internal/os/SafeZipPathValidatorCallbackTest.java
+++ /dev/null
@@ -1,244 +0,0 @@
-/*
- * Copyright (C) 2022 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.
- */
-
-package com.android.internal.os;
-
-import static org.junit.Assert.assertThrows;
-
-import android.compat.testing.PlatformCompatChangeRule;
-
-import androidx.test.runner.AndroidJUnit4;
-
-import libcore.junit.util.compat.CoreCompatChangeRule.DisableCompatChanges;
-import libcore.junit.util.compat.CoreCompatChangeRule.EnableCompatChanges;
-
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TestRule;
-import org.junit.runner.RunWith;
-
-import java.io.ByteArrayInputStream;
-import java.io.ByteArrayOutputStream;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.io.OutputStream;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipException;
-import java.util.zip.ZipFile;
-import java.util.zip.ZipInputStream;
-import java.util.zip.ZipOutputStream;
-
-/**
- * Test SafeZipPathCallback.
- */
-@RunWith(AndroidJUnit4.class)
-public class SafeZipPathValidatorCallbackTest {
- @Rule
- public TestRule mCompatChangeRule = new PlatformCompatChangeRule();
-
- @Before
- public void setUp() {
- RuntimeInit.initZipPathValidatorCallback();
- }
-
- @Test
- @EnableCompatChanges({SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL})
- public void testNewZipFile_whenZipFileHasDangerousEntriesAndChangeEnabled_throws()
- throws Exception {
- final String[] dangerousEntryNames = {
- "../foo.bar",
- "foo/../bar.baz",
- "foo/../../bar.baz",
- "foo.bar/..",
- "foo.bar/../",
- "..",
- "../",
- "/foo",
- };
- for (String entryName : dangerousEntryNames) {
- final File tempFile = File.createTempFile("smdc", "zip");
- try {
- writeZipFileOutputStreamWithEmptyEntry(tempFile, entryName);
-
- assertThrows(
- "ZipException expected for entry: " + entryName,
- ZipException.class,
- () -> {
- new ZipFile(tempFile);
- });
- } finally {
- tempFile.delete();
- }
- }
- }
-
- @Test
- @EnableCompatChanges({SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL})
- public void
- testZipInputStreamGetNextEntry_whenZipFileHasDangerousEntriesAndChangeEnabled_throws()
- throws Exception {
- final String[] dangerousEntryNames = {
- "../foo.bar",
- "foo/../bar.baz",
- "foo/../../bar.baz",
- "foo.bar/..",
- "foo.bar/../",
- "..",
- "../",
- "/foo",
- };
- for (String entryName : dangerousEntryNames) {
- byte[] badZipBytes = getZipBytesFromZipOutputStreamWithEmptyEntry(entryName);
- try (ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(badZipBytes))) {
- assertThrows(
- "ZipException expected for entry: " + entryName,
- ZipException.class,
- () -> {
- zis.getNextEntry();
- });
- }
- }
- }
-
- @Test
- @EnableCompatChanges({SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL})
- public void testNewZipFile_whenZipFileHasNormalEntriesAndChangeEnabled_doesNotThrow()
- throws Exception {
- final String[] normalEntryNames = {
- "foo", "foo.bar", "foo..bar",
- };
- for (String entryName : normalEntryNames) {
- final File tempFile = File.createTempFile("smdc", "zip");
- try {
- writeZipFileOutputStreamWithEmptyEntry(tempFile, entryName);
- try {
- new ZipFile((tempFile));
- } catch (ZipException e) {
- throw new AssertionError("ZipException not expected for entry: " + entryName);
- }
- } finally {
- tempFile.delete();
- }
- }
- }
-
- @Test
- @DisableCompatChanges({SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL})
- public void
- testZipInputStreamGetNextEntry_whenZipFileHasNormalEntriesAndChangeEnabled_doesNotThrow()
- throws Exception {
- final String[] normalEntryNames = {
- "foo", "foo.bar", "foo..bar",
- };
- for (String entryName : normalEntryNames) {
- byte[] zipBytes = getZipBytesFromZipOutputStreamWithEmptyEntry(entryName);
- try {
- ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(zipBytes));
- zis.getNextEntry();
- } catch (ZipException e) {
- throw new AssertionError("ZipException not expected for entry: " + entryName);
- }
- }
- }
-
- @Test
- @DisableCompatChanges({SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL})
- public void
- testNewZipFile_whenZipFileHasNormalAndDangerousEntriesAndChangeDisabled_doesNotThrow()
- throws Exception {
- final String[] entryNames = {
- "../foo.bar",
- "foo/../bar.baz",
- "foo/../../bar.baz",
- "foo.bar/..",
- "foo.bar/../",
- "..",
- "../",
- "/foo",
- "foo",
- "foo.bar",
- "foo..bar",
- };
- for (String entryName : entryNames) {
- final File tempFile = File.createTempFile("smdc", "zip");
- try {
- writeZipFileOutputStreamWithEmptyEntry(tempFile, entryName);
- try {
- new ZipFile((tempFile));
- } catch (ZipException e) {
- throw new AssertionError("ZipException not expected for entry: " + entryName);
- }
- } finally {
- tempFile.delete();
- }
- }
- }
-
- @Test
- @DisableCompatChanges({SafeZipPathValidatorCallback.VALIDATE_ZIP_PATH_FOR_PATH_TRAVERSAL})
- public void
- testZipInputStreamGetNextEntry_whenZipFileHasNormalAndDangerousEntriesAndChangeDisabled_doesNotThrow()
- throws Exception {
- final String[] entryNames = {
- "../foo.bar",
- "foo/../bar.baz",
- "foo/../../bar.baz",
- "foo.bar/..",
- "foo.bar/../",
- "..",
- "../",
- "/foo",
- "foo",
- "foo.bar",
- "foo..bar",
- };
- for (String entryName : entryNames) {
- byte[] zipBytes = getZipBytesFromZipOutputStreamWithEmptyEntry(entryName);
- try {
- ZipInputStream zis = new ZipInputStream(new ByteArrayInputStream(zipBytes));
- zis.getNextEntry();
- } catch (ZipException e) {
- throw new AssertionError("ZipException not expected for entry: " + entryName);
- }
- }
- }
-
- private void writeZipFileOutputStreamWithEmptyEntry(File tempFile, String entryName)
- throws IOException {
- FileOutputStream tempFileStream = new FileOutputStream(tempFile);
- writeZipOutputStreamWithEmptyEntry(tempFileStream, entryName);
- tempFileStream.close();
- }
-
- private byte[] getZipBytesFromZipOutputStreamWithEmptyEntry(String entryName)
- throws IOException {
- ByteArrayOutputStream bos = new ByteArrayOutputStream();
- writeZipOutputStreamWithEmptyEntry(bos, entryName);
- return bos.toByteArray();
- }
-
- private void writeZipOutputStreamWithEmptyEntry(OutputStream os, String entryName)
- throws IOException {
- ZipOutputStream zos = new ZipOutputStream(os);
- ZipEntry entry = new ZipEntry(entryName);
- zos.putNextEntry(entry);
- zos.write(new byte[2]);
- zos.closeEntry();
- zos.close();
- }
-}