Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b49caa85 authored by Azhara Assanova's avatar Azhara Assanova
Browse files

Move initZipPathValidator() into ActivityThread

This CL is just an internal cherry-pick of aosp/2428074 because of automerger conflicts.

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: I539465d818e78a7f1dd4dddbe345331836e09caf
parent ca9e2463
Loading
Loading
Loading
Loading
+21 −0
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import static android.window.ConfigurationHelper.isDifferentDisplay;
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;
@@ -49,6 +50,7 @@ import android.app.assist.AssistStructure;
import android.app.backup.BackupAgent;
import android.app.backup.BackupAnnotations.BackupDestination;
import android.app.backup.BackupAnnotations.OperationType;
import android.app.compat.CompatChanges;
import android.app.servertransaction.ActivityLifecycleItem;
import android.app.servertransaction.ActivityLifecycleItem.LifecycleState;
import android.app.servertransaction.ActivityRelaunchItem;
@@ -206,6 +208,7 @@ import com.android.internal.content.ReferrerIntent;
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;
@@ -219,6 +222,7 @@ import dalvik.system.AppSpecializationHooks;
import dalvik.system.CloseGuard;
import dalvik.system.VMDebug;
import dalvik.system.VMRuntime;
import dalvik.system.ZipPathValidator;

import libcore.io.ForwardingOs;
import libcore.io.IoUtils;
@@ -6703,6 +6707,11 @@ public final class ActivityThread extends ClientTransactionHandler
        // 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);
@@ -7070,6 +7079,18 @@ public final class ActivityThread extends ClientTransactionHandler
        }
    }

    /**
     * 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) {
+0 −26
Original line number Diff line number Diff line
@@ -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 com.android.internal.logging.AndroidConfig;

import dalvik.system.RuntimeHooks;
import dalvik.system.VMRuntime;
import dalvik.system.ZipPathValidator;

import libcore.content.type.MimeMap;

@@ -265,30 +260,9 @@ public class RuntimeInit {
         */
        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)".
+0 −244
Original line number Diff line number Diff line
/*
 * 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();
    }
}