From bef47a35776cf1d5ae9124b3a3823d1b950d0b80 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Tue, 18 May 2021 15:45:48 -0700 Subject: [PATCH 1/2] StringBlock incremental hardening default values When a string cannot be retrieved from a StringBlock due to pages of the StringPool missing, rather than throwing an IndexOutOfBoundsException, compute a default value rather that abides by the API contacts of the functions that use the StringBlock. IndexOutOfBoundsExceptions are not really documented as a possible exception that can be thrown from APIs that use StringBlock because it indicates that the binary resource table was built incorrectly. Returning default values is much less likely to break applications that fetch the resources of incrementally installed applications than throwing IndexOutOfBoundsExceptions where they would normally not be expected. Bug: 188174746 Test: atest ResourcesHardeningTest Change-Id: I58fe754fb446fefc031ddba19e290830fdd6d015 --- core/java/android/content/res/ApkAssets.java | 3 +- .../android/content/res/AssetManager.java | 9 +++- .../java/android/content/res/StringBlock.java | 29 +++++++++++- core/java/android/content/res/TypedArray.java | 1 + core/java/android/content/res/XmlBlock.java | 44 ++++++++++++++----- core/jni/android_util_StringBlock.cpp | 8 ++-- 6 files changed, 74 insertions(+), 20 deletions(-) diff --git a/core/java/android/content/res/ApkAssets.java b/core/java/android/content/res/ApkAssets.java index f3783e4a13286..224ff14c5df47 100644 --- a/core/java/android/content/res/ApkAssets.java +++ b/core/java/android/content/res/ApkAssets.java @@ -334,13 +334,14 @@ public final class ApkAssets { } } + @Nullable CharSequence getStringFromPool(int idx) { if (mStringBlock == null) { return null; } synchronized (this) { - return mStringBlock.get(idx); + return mStringBlock.getSequence(idx); } } diff --git a/core/java/android/content/res/AssetManager.java b/core/java/android/content/res/AssetManager.java index 13433dc7979f4..d50404e94544b 100644 --- a/core/java/android/content/res/AssetManager.java +++ b/core/java/android/content/res/AssetManager.java @@ -550,7 +550,9 @@ public final class AssetManager implements AutoCloseable { outValue.changingConfigurations); if (outValue.type == TypedValue.TYPE_STRING) { - outValue.string = getPooledStringForCookie(cookie, outValue.data); + if ((outValue.string = getPooledStringForCookie(cookie, outValue.data)) == null) { + return false; + } } return true; } @@ -731,7 +733,9 @@ public final class AssetManager implements AutoCloseable { outValue.changingConfigurations); if (outValue.type == TypedValue.TYPE_STRING) { - outValue.string = getPooledStringForCookie(cookie, outValue.data); + if ((outValue.string = getPooledStringForCookie(cookie, outValue.data)) == null) { + return false; + } } return true; } @@ -833,6 +837,7 @@ public final class AssetManager implements AutoCloseable { } } + @Nullable CharSequence getPooledStringForCookie(int cookie, int id) { // Cookies map to ApkAssets starting at 1. return getApkAssets()[cookie - 1].getStringFromPool(id); diff --git a/core/java/android/content/res/StringBlock.java b/core/java/android/content/res/StringBlock.java index 8f3f77b997392..5bc235f0eebaa 100644 --- a/core/java/android/content/res/StringBlock.java +++ b/core/java/android/content/res/StringBlock.java @@ -16,6 +16,8 @@ package android.content.res; +import android.annotation.NonNull; +import android.annotation.Nullable; import android.compat.annotation.UnsupportedAppUsage; import android.graphics.Color; import android.graphics.Paint; @@ -86,8 +88,19 @@ public final class StringBlock implements Closeable { + ": " + nativeGetSize(mNative)); } + /** + * @deprecated use {@link #getSequence(int)} which can return null when a string cannot be found + * due to incremental installation. + */ + @Deprecated @UnsupportedAppUsage public CharSequence get(int idx) { + CharSequence seq = getSequence(idx); + return seq == null ? "" : seq; + } + + @Nullable + public CharSequence getSequence(int idx) { synchronized (this) { if (mStrings != null) { CharSequence res = mStrings[idx]; @@ -108,6 +121,9 @@ public final class StringBlock implements Closeable { } } String str = nativeGetString(mNative, idx); + if (str == null) { + return null; + } CharSequence res = str; int[] style = nativeGetStyle(mNative, idx); if (localLOGV) Log.v(TAG, "Got string: " + str); @@ -133,6 +149,9 @@ public final class StringBlock implements Closeable { } String styleTag = nativeGetString(mNative, styleId); + if (styleTag == null) { + return null; + } if (styleTag.equals("b")) { mStyleIDs.boldId = styleId; @@ -161,8 +180,10 @@ public final class StringBlock implements Closeable { res = applyStyles(str, style, mStyleIDs); } - if (mStrings != null) mStrings[idx] = res; - else mSparseStrings.put(idx, res); + if (res != null) { + if (mStrings != null) mStrings[idx] = res; + else mSparseStrings.put(idx, res); + } return res; } } @@ -203,6 +224,7 @@ public final class StringBlock implements Closeable { private int marqueeId = -1; } + @Nullable private CharSequence applyStyles(String str, int[] style, StyleIDs ids) { if (style.length == 0) return str; @@ -260,6 +282,9 @@ public final class StringBlock implements Closeable { Spannable.SPAN_INCLUSIVE_INCLUSIVE); } else { String tag = nativeGetString(mNative, type); + if (tag == null) { + return null; + } if (tag.startsWith("font;")) { String sub; diff --git a/core/java/android/content/res/TypedArray.java b/core/java/android/content/res/TypedArray.java index 2a349e9a4600d..d6f55d65ead55 100644 --- a/core/java/android/content/res/TypedArray.java +++ b/core/java/android/content/res/TypedArray.java @@ -1376,6 +1376,7 @@ public class TypedArray implements AutoCloseable { return true; } + @Nullable private CharSequence loadStringValueAt(int index) { final int[] data = mData; final int cookie = data[index + STYLE_ASSET_COOKIE]; diff --git a/core/java/android/content/res/XmlBlock.java b/core/java/android/content/res/XmlBlock.java index b0291ce6d8429..53dfed374fbca 100644 --- a/core/java/android/content/res/XmlBlock.java +++ b/core/java/android/content/res/XmlBlock.java @@ -19,6 +19,7 @@ package android.content.res; import static android.content.res.Resources.ID_NULL; import android.annotation.AnyRes; +import android.annotation.NonNull; import android.annotation.Nullable; import android.compat.annotation.UnsupportedAppUsage; import android.os.Build; @@ -161,9 +162,10 @@ public final class XmlBlock implements AutoCloseable { public int getDepth() { return mDepth; } + @Nullable public String getText() { int id = nativeGetText(mParseState); - return id >= 0 ? mStrings.get(id).toString() : null; + return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null; } public int getLineNumber() { return nativeGetLineNumber(mParseState); @@ -189,25 +191,29 @@ public final class XmlBlock implements AutoCloseable { } return chars; } + @Nullable public String getNamespace() { int id = nativeGetNamespace(mParseState); - return id >= 0 ? mStrings.get(id).toString() : ""; + return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : ""; } + @Nullable public String getName() { int id = nativeGetName(mParseState); - return id >= 0 ? mStrings.get(id).toString() : null; + return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null; } + @NonNull public String getAttributeNamespace(int index) { int id = nativeGetAttributeNamespace(mParseState, index); if (DEBUG) System.out.println("getAttributeNamespace of " + index + " = " + id); - if (id >= 0) return mStrings.get(id).toString(); + if (id >= 0) return getSequenceString(mStrings.getSequence(id)); else if (id == -1) return ""; throw new IndexOutOfBoundsException(String.valueOf(index)); } + @NonNull public String getAttributeName(int index) { int id = nativeGetAttributeName(mParseState, index); if (DEBUG) System.out.println("getAttributeName of " + index + " = " + id); - if (id >= 0) return mStrings.get(id).toString(); + if (id >= 0) return getSequenceString(mStrings.getSequence(id)); throw new IndexOutOfBoundsException(String.valueOf(index)); } public String getAttributePrefix(int index) { @@ -220,10 +226,11 @@ public final class XmlBlock implements AutoCloseable { public int getAttributeCount() { return mEventType == START_TAG ? nativeGetAttributeCount(mParseState) : -1; } + @NonNull public String getAttributeValue(int index) { int id = nativeGetAttributeStringValue(mParseState, index); if (DEBUG) System.out.println("getAttributeValue of " + index + " = " + id); - if (id >= 0) return mStrings.get(id).toString(); + if (id >= 0) return getSequenceString(mStrings.getSequence(id)); // May be some other type... check and try to convert if so. int t = nativeGetAttributeDataType(mParseState, index); @@ -390,7 +397,7 @@ public final class XmlBlock implements AutoCloseable { int v = nativeGetAttributeData(mParseState, idx); if (t == TypedValue.TYPE_STRING) { return XmlUtils.convertValueToList( - mStrings.get(v), options, defaultValue); + mStrings.getSequence(v), options, defaultValue); } return v; } @@ -444,14 +451,15 @@ public final class XmlBlock implements AutoCloseable { } throw new RuntimeException("not a float!"); } - + @Nullable public String getIdAttribute() { int id = nativeGetIdAttribute(mParseState); - return id >= 0 ? mStrings.get(id).toString() : null; + return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null; } + @Nullable public String getClassAttribute() { int id = nativeGetClassAttribute(mParseState); - return id >= 0 ? mStrings.get(id).toString() : null; + return id >= 0 ? getSequenceString(mStrings.getSequence(id)) : null; } public int getIdAttributeResourceValue(int defaultValue) { @@ -463,6 +471,17 @@ public final class XmlBlock implements AutoCloseable { return nativeGetStyleAttribute(mParseState); } + private String getSequenceString(@Nullable CharSequence str) { + if (str == null) { + // A value of null retrieved from a StringPool indicates that retrieval of the + // string failed due to incremental installation. The presence of all the XmlBlock + // data is verified when it is created, so this exception must not be possible. + throw new IllegalStateException("Retrieving a string from the StringPool of an" + + " XmlBlock should never fail"); + } + return str.toString(); + } + public void close() { synchronized (mBlock) { if (mParseState != 0) { @@ -472,13 +491,14 @@ public final class XmlBlock implements AutoCloseable { } } } - + protected void finalize() throws Throwable { close(); } + @Nullable /*package*/ final CharSequence getPooledString(int id) { - return mStrings.get(id); + return mStrings.getSequence(id); } @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553) diff --git a/core/jni/android_util_StringBlock.cpp b/core/jni/android_util_StringBlock.cpp index d94e0841003f2..dd7df26ee8819 100644 --- a/core/jni/android_util_StringBlock.cpp +++ b/core/jni/android_util_StringBlock.cpp @@ -72,7 +72,7 @@ static jstring android_content_StringBlock_nativeGetString(JNIEnv* env, jobject ResStringPool* osb = reinterpret_cast(token); if (osb == NULL) { jniThrowNullPointerException(env, NULL); - return 0; + return NULL; } if (auto str8 = osb->string8At(idx); str8.has_value()) { @@ -80,9 +80,11 @@ static jstring android_content_StringBlock_nativeGetString(JNIEnv* env, jobject } auto str = osb->stringAt(idx); - if (UNLIKELY(!str.has_value())) { + if (IsIOError(str)) { + return NULL; + } else if (UNLIKELY(!str.has_value())) { jniThrowException(env, "java/lang/IndexOutOfBoundsException", NULL); - return 0; + return NULL; } return env->NewString((const jchar*)str->data(), str->size()); From c0416698dbaaeba7b706c9eca59e2ba0cab45377 Mon Sep 17 00:00:00 2001 From: Ryan Mitchell Date: Tue, 11 May 2021 12:21:29 -0700 Subject: [PATCH 2/2] Disable incremental hardening on own resources When an application is incrementally installed, and a resources operation fails due to the resources not being fully present, the app should crash instead of swallowing the error and returning default values to not alter the experience of using the application. Disable IncFsFileMap protections on ApkAssets that are a part of the application that is running (base and splits). Bug: 187220960 Test: atest ResourcesHardeningTest Change-Id: Ibc67aca688720f983c7c656f404593285a54999b --- cmds/idmap2/libidmap2/ResourceContainer.cpp | 2 +- cmds/idmap2/tests/XmlParserTests.cpp | 2 +- core/java/android/app/LoadedApk.java | 4 ++ core/java/android/app/ResourcesManager.java | 51 +++++++++++++++++-- core/java/android/content/res/ApkAssets.java | 6 +++ core/jni/android_content_res_ApkAssets.cpp | 37 ++++++++------ libs/androidfw/ApkAssets.cpp | 4 +- libs/androidfw/AssetsProvider.cpp | 16 +++--- libs/androidfw/TEST_MAPPING | 3 ++ .../include/androidfw/AssetsProvider.h | 9 +++- libs/androidfw/include/androidfw/LoadedArsc.h | 4 ++ 11 files changed, 106 insertions(+), 32 deletions(-) diff --git a/cmds/idmap2/libidmap2/ResourceContainer.cpp b/cmds/idmap2/libidmap2/ResourceContainer.cpp index 9147ccaaa17aa..c147f6a6024bb 100644 --- a/cmds/idmap2/libidmap2/ResourceContainer.cpp +++ b/cmds/idmap2/libidmap2/ResourceContainer.cpp @@ -323,7 +323,7 @@ ApkResourceContainer::ApkResourceContainer(std::unique_ptr zi Result> ApkResourceContainer::FromPath( const std::string& path) { - auto zip_assets = ZipAssetsProvider::Create(path); + auto zip_assets = ZipAssetsProvider::Create(path, 0 /* flags */); if (zip_assets == nullptr) { return Error("failed to load zip assets"); } diff --git a/cmds/idmap2/tests/XmlParserTests.cpp b/cmds/idmap2/tests/XmlParserTests.cpp index eaf10a7d9282f..016c5c3a01a5e 100644 --- a/cmds/idmap2/tests/XmlParserTests.cpp +++ b/cmds/idmap2/tests/XmlParserTests.cpp @@ -26,7 +26,7 @@ namespace android::idmap2 { Result CreateTestParser(const std::string& test_file) { - auto zip = ZipAssetsProvider::Create(GetTestDataPath() + "/target/target.apk"); + auto zip = ZipAssetsProvider::Create(GetTestDataPath() + "/target/target.apk", 0 /* flags */); if (zip == nullptr) { return Error("Failed to open zip file"); } diff --git a/core/java/android/app/LoadedApk.java b/core/java/android/app/LoadedApk.java index 0733a3e7c51cf..90a6f32b2344d 100644 --- a/core/java/android/app/LoadedApk.java +++ b/core/java/android/app/LoadedApk.java @@ -1289,6 +1289,10 @@ public final class LoadedApk { throw new AssertionError("null split not found"); } + if (Process.myUid() == mApplicationInfo.uid) { + ResourcesManager.getInstance().initializeApplicationPaths(mResDir, splitPaths); + } + mResources = ResourcesManager.getInstance().getResources(null, mResDir, splitPaths, mLegacyOverlayDirs, mOverlayPaths, mApplicationInfo.sharedLibraryFiles, null, null, getCompatibilityInfo(), diff --git a/core/java/android/app/ResourcesManager.java b/core/java/android/app/ResourcesManager.java index 792336d9be9ec..f28c760d54d95 100644 --- a/core/java/android/app/ResourcesManager.java +++ b/core/java/android/app/ResourcesManager.java @@ -39,6 +39,7 @@ import android.os.IBinder; import android.os.Process; import android.os.Trace; import android.util.ArrayMap; +import android.util.ArraySet; import android.util.DisplayMetrics; import android.util.Log; import android.util.Pair; @@ -260,6 +261,12 @@ public class ResourcesManager { */ private final UpdateHandler mUpdateCallbacks = new UpdateHandler(); + /** + * The set of APK paths belonging to this process. This is used to disable incremental + * installation crash protections on these APKs so the app either behaves as expects or crashes. + */ + private final ArraySet mApplicationOwnedApks = new ArraySet<>(); + @UnsupportedAppUsage public ResourcesManager() { } @@ -424,6 +431,32 @@ public class ResourcesManager { } } + /** + * Initializes the set of APKs owned by the application running in this process. + */ + public void initializeApplicationPaths(@NonNull String sourceDir, + @Nullable String[] splitDirs) { + synchronized (mLock) { + if (mApplicationOwnedApks.isEmpty()) { + addApplicationPathsLocked(sourceDir, splitDirs); + } + } + } + + /** + * Updates the set of APKs owned by the application running in this process. + * + * This method only appends to the set of APKs owned by this process because the previous APKs + * paths still belong to the application running in this process. + */ + private void addApplicationPathsLocked(@NonNull String sourceDir, + @Nullable String[] splitDirs) { + mApplicationOwnedApks.add(sourceDir); + if (splitDirs != null) { + mApplicationOwnedApks.addAll(Arrays.asList(splitDirs)); + } + } + private static String overlayPathToIdmapPath(String path) { return "/data/resource-cache/" + path.substring(1).replace('/', '@') + "@idmap"; } @@ -445,13 +478,17 @@ public class ResourcesManager { } } - // We must load this from disk. + int flags = 0; + if (key.sharedLib) { + flags |= ApkAssets.PROPERTY_DYNAMIC; + } + if (mApplicationOwnedApks.contains(key.path)) { + flags |= ApkAssets.PROPERTY_DISABLE_INCREMENTAL_HARDENING; + } if (key.overlay) { - apkAssets = ApkAssets.loadOverlayFromPath(overlayPathToIdmapPath(key.path), - 0 /*flags*/); + apkAssets = ApkAssets.loadOverlayFromPath(overlayPathToIdmapPath(key.path), flags); } else { - apkAssets = ApkAssets.loadFromPath(key.path, - key.sharedLib ? ApkAssets.PROPERTY_DYNAMIC : 0); + apkAssets = ApkAssets.loadFromPath(key.path, flags); } synchronized (mLock) { @@ -1437,6 +1474,10 @@ public class ResourcesManager { String[] copiedResourceDirs = combinedOverlayPaths(appInfo.resourceDirs, appInfo.overlayPaths); + if (appInfo.uid == myUid) { + addApplicationPathsLocked(baseCodePath, copiedSplitDirs); + } + final ArrayMap updatedResourceKeys = new ArrayMap<>(); final int implCount = mResourceImpls.size(); for (int i = 0; i < implCount; i++) { diff --git a/core/java/android/content/res/ApkAssets.java b/core/java/android/content/res/ApkAssets.java index 224ff14c5df47..6fd2d05ad1357 100644 --- a/core/java/android/content/res/ApkAssets.java +++ b/core/java/android/content/res/ApkAssets.java @@ -69,6 +69,12 @@ public final class ApkAssets { */ private static final int PROPERTY_OVERLAY = 1 << 3; + /** + * The apk assets is owned by the application running in this process and incremental crash + * protections for this APK must be disabled. + */ + public static final int PROPERTY_DISABLE_INCREMENTAL_HARDENING = 1 << 4; + /** Flags that change the behavior of loaded apk assets. */ @IntDef(prefix = { "PROPERTY_" }, value = { PROPERTY_SYSTEM, diff --git a/core/jni/android_content_res_ApkAssets.cpp b/core/jni/android_content_res_ApkAssets.cpp index 9c8daec7919a4..1be84282f6db6 100644 --- a/core/jni/android_content_res_ApkAssets.cpp +++ b/core/jni/android_content_res_ApkAssets.cpp @@ -212,10 +212,11 @@ static jlong NativeLoad(JNIEnv* env, jclass /*clazz*/, const format_type_t forma std::unique_ptr apk_assets; switch (format) { case FORMAT_APK: { - auto assets = MultiAssetsProvider::Create(std::move(loader_assets), - ZipAssetsProvider::Create(path.c_str())); - apk_assets = ApkAssets::Load(std::move(assets), property_flags); - break; + auto assets = MultiAssetsProvider::Create(std::move(loader_assets), + ZipAssetsProvider::Create(path.c_str(), + property_flags)); + apk_assets = ApkAssets::Load(std::move(assets), property_flags); + break; } case FORMAT_IDMAP: apk_assets = ApkAssets::LoadOverlay(path.c_str(), property_flags); @@ -271,11 +272,13 @@ static jlong NativeLoadFromFd(JNIEnv* env, jclass /*clazz*/, const format_type_t std::unique_ptr apk_assets; switch (format) { case FORMAT_APK: { - auto assets = MultiAssetsProvider::Create( - std::move(loader_assets), - ZipAssetsProvider::Create(std::move(dup_fd), friendly_name_utf8.c_str())); - apk_assets = ApkAssets::Load(std::move(assets), property_flags); - break; + auto assets = + MultiAssetsProvider::Create(std::move(loader_assets), + ZipAssetsProvider::Create(std::move(dup_fd), + friendly_name_utf8.c_str(), + property_flags)); + apk_assets = ApkAssets::Load(std::move(assets), property_flags); + break; } case FORMAT_ARSC: apk_assets = ApkAssets::LoadTable( @@ -336,12 +339,16 @@ static jlong NativeLoadFromFdOffset(JNIEnv* env, jclass /*clazz*/, const format_ std::unique_ptr apk_assets; switch (format) { case FORMAT_APK: { - auto assets = MultiAssetsProvider::Create( - std::move(loader_assets), - ZipAssetsProvider::Create(std::move(dup_fd), friendly_name_utf8.c_str(), - static_cast(offset), static_cast(length))); - apk_assets = ApkAssets::Load(std::move(assets), property_flags); - break; + auto assets = + MultiAssetsProvider::Create(std::move(loader_assets), + ZipAssetsProvider::Create(std::move(dup_fd), + friendly_name_utf8.c_str(), + property_flags, + static_cast(offset), + static_cast( + length))); + apk_assets = ApkAssets::Load(std::move(assets), property_flags); + break; } case FORMAT_ARSC: apk_assets = ApkAssets::LoadTable( diff --git a/libs/androidfw/ApkAssets.cpp b/libs/androidfw/ApkAssets.cpp index 26d836328b54e..2beb33abe782c 100755 --- a/libs/androidfw/ApkAssets.cpp +++ b/libs/androidfw/ApkAssets.cpp @@ -40,7 +40,7 @@ ApkAssets::ApkAssets(std::unique_ptr resources_asset, loaded_idmap_(std::move(loaded_idmap)) {} std::unique_ptr ApkAssets::Load(const std::string& path, package_property_t flags) { - return Load(ZipAssetsProvider::Create(path), flags); + return Load(ZipAssetsProvider::Create(path, flags), flags); } std::unique_ptr ApkAssets::LoadFromFd(base::unique_fd fd, @@ -91,7 +91,7 @@ std::unique_ptr ApkAssets::LoadOverlay(const std::string& idmap_path, overlay_assets = EmptyAssetsProvider::Create(overlay_path); } else { // The overlay should be an APK. - overlay_assets = ZipAssetsProvider::Create(overlay_path); + overlay_assets = ZipAssetsProvider::Create(overlay_path, flags); } if (overlay_assets == nullptr) { return {}; diff --git a/libs/androidfw/AssetsProvider.cpp b/libs/androidfw/AssetsProvider.cpp index 6c7a253072475..bce34d37c90bc 100644 --- a/libs/androidfw/AssetsProvider.cpp +++ b/libs/androidfw/AssetsProvider.cpp @@ -85,12 +85,14 @@ const std::string& ZipAssetsProvider::PathOrDebugName::GetDebugName() const { } ZipAssetsProvider::ZipAssetsProvider(ZipArchiveHandle handle, PathOrDebugName&& path, - time_t last_mod_time) + package_property_t flags, time_t last_mod_time) : zip_handle_(handle, ::CloseArchive), name_(std::forward(path)), + flags_(flags), last_mod_time_(last_mod_time) {} -std::unique_ptr ZipAssetsProvider::Create(std::string path) { +std::unique_ptr ZipAssetsProvider::Create(std::string path, + package_property_t flags) { ZipArchiveHandle handle; if (int32_t result = OpenArchive(path.c_str(), &handle); result != 0) { LOG(ERROR) << "Failed to open APK '" << path << "': " << ::ErrorCodeString(result); @@ -109,11 +111,12 @@ std::unique_ptr ZipAssetsProvider::Create(std::string path) { return std::unique_ptr( new ZipAssetsProvider(handle, PathOrDebugName{std::move(path), - true /* is_path */}, sb.st_mtime)); + true /* is_path */}, flags, sb.st_mtime)); } std::unique_ptr ZipAssetsProvider::Create(base::unique_fd fd, std::string friendly_name, + package_property_t flags, off64_t offset, off64_t len) { ZipArchiveHandle handle; @@ -140,7 +143,7 @@ std::unique_ptr ZipAssetsProvider::Create(base::unique_fd fd, return std::unique_ptr( new ZipAssetsProvider(handle, PathOrDebugName{std::move(friendly_name), - false /* is_path */}, sb.st_mtime)); + false /* is_path */}, flags, sb.st_mtime)); } std::unique_ptr ZipAssetsProvider::OpenInternal(const std::string& path, @@ -161,10 +164,11 @@ std::unique_ptr ZipAssetsProvider::OpenInternal(const std::string& path, const int fd = GetFileDescriptor(zip_handle_.get()); const off64_t fd_offset = GetFileDescriptorOffset(zip_handle_.get()); + const bool incremental_hardening = (flags_ & PROPERTY_DISABLE_INCREMENTAL_HARDENING) == 0U; incfs::IncFsFileMap asset_map; if (entry.method == kCompressDeflated) { if (!asset_map.Create(fd, entry.offset + fd_offset, entry.compressed_length, - name_.GetDebugName().c_str())) { + name_.GetDebugName().c_str(), incremental_hardening)) { LOG(ERROR) << "Failed to mmap file '" << path << "' in APK '" << name_.GetDebugName() << "'"; return {}; @@ -181,7 +185,7 @@ std::unique_ptr ZipAssetsProvider::OpenInternal(const std::string& path, } if (!asset_map.Create(fd, entry.offset + fd_offset, entry.uncompressed_length, - name_.GetDebugName().c_str())) { + name_.GetDebugName().c_str(), incremental_hardening)) { LOG(ERROR) << "Failed to mmap file '" << path << "' in APK '" << name_.GetDebugName() << "'"; return {}; } diff --git a/libs/androidfw/TEST_MAPPING b/libs/androidfw/TEST_MAPPING index 766714cd76941..8a5c06d1eef94 100644 --- a/libs/androidfw/TEST_MAPPING +++ b/libs/androidfw/TEST_MAPPING @@ -2,6 +2,9 @@ "presubmit": [ { "name": "CtsResourcesLoaderTests" + }, + { + "name": "ResourcesHardeningTest" } ] } \ No newline at end of file diff --git a/libs/androidfw/include/androidfw/AssetsProvider.h b/libs/androidfw/include/androidfw/AssetsProvider.h index ec51c65053bf2..966ec74c1786a 100644 --- a/libs/androidfw/include/androidfw/AssetsProvider.h +++ b/libs/androidfw/include/androidfw/AssetsProvider.h @@ -80,9 +80,12 @@ struct AssetsProvider { // Supplies assets from a zip archive. struct ZipAssetsProvider : public AssetsProvider { - static std::unique_ptr Create(std::string path); + static std::unique_ptr Create(std::string path, + package_property_t flags); + static std::unique_ptr Create(base::unique_fd fd, std::string friendly_name, + package_property_t flags, off64_t offset = 0, off64_t len = kUnknownLength); @@ -101,7 +104,8 @@ struct ZipAssetsProvider : public AssetsProvider { private: struct PathOrDebugName; - ZipAssetsProvider(ZipArchive* handle, PathOrDebugName&& path, time_t last_mod_time); + ZipAssetsProvider(ZipArchive* handle, PathOrDebugName&& path, package_property_t flags, + time_t last_mod_time); struct PathOrDebugName { PathOrDebugName(std::string&& value, bool is_path); @@ -119,6 +123,7 @@ struct ZipAssetsProvider : public AssetsProvider { std::unique_ptr zip_handle_; PathOrDebugName name_; + package_property_t flags_; time_t last_mod_time_; }; diff --git a/libs/androidfw/include/androidfw/LoadedArsc.h b/libs/androidfw/include/androidfw/LoadedArsc.h index 9bbdede562930..b3d6a4dcb955c 100644 --- a/libs/androidfw/include/androidfw/LoadedArsc.h +++ b/libs/androidfw/include/androidfw/LoadedArsc.h @@ -92,6 +92,10 @@ enum : package_property_t { // The package is a RRO. PROPERTY_OVERLAY = 1U << 3U, + + // The apk assets is owned by the application running in this process and incremental crash + // protections for this APK must be disabled. + PROPERTY_DISABLE_INCREMENTAL_HARDENING = 1U << 4U, }; struct OverlayableInfo {