Have aapt2 check library names the same as package manager
Bug: 231297692
Test: Manual
Change-Id: I11a660969443aa90cf6b51a0947accca4231310f
diff --git a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
index 6762e45..2c81987 100644
--- a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
+++ b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
@@ -17,6 +17,7 @@
#define LOG_TAG "NativeLibraryHelper"
//#define LOG_NDEBUG 0
+#include <androidfw/ApkParsing.h>
#include <androidfw/ZipFileRO.h>
#include <androidfw/ZipUtils.h>
#include <errno.h>
@@ -37,15 +38,6 @@
#include "core_jni_helpers.h"
-#define APK_LIB "lib/"
-#define APK_LIB_LEN (sizeof(APK_LIB) - 1)
-
-#define LIB_PREFIX "/lib"
-#define LIB_PREFIX_LEN (sizeof(LIB_PREFIX) - 1)
-
-#define LIB_SUFFIX ".so"
-#define LIB_SUFFIX_LEN (sizeof(LIB_SUFFIX) - 1)
-
#define RS_BITCODE_SUFFIX ".bc"
#define TMP_FILE_PATTERN "/tmp.XXXXXX"
@@ -66,39 +58,6 @@
typedef install_status_t (*iterFunc)(JNIEnv*, void*, ZipFileRO*, ZipEntryRO, const char*);
-// Equivalent to android.os.FileUtils.isFilenameSafe
-static bool
-isFilenameSafe(const char* filename)
-{
- off_t offset = 0;
- for (;;) {
- switch (*(filename + offset)) {
- case 0:
- // Null.
- // If we've reached the end, all the other characters are good.
- return true;
-
- case 'A' ... 'Z':
- case 'a' ... 'z':
- case '0' ... '9':
- case '+':
- case ',':
- case '-':
- case '.':
- case '/':
- case '=':
- case '_':
- offset++;
- break;
-
- default:
- // We found something that is not good.
- return false;
- }
- }
- // Should not reach here.
-}
-
static bool
isFileDifferent(const char* filePath, uint32_t fileSize, time_t modifiedTime,
uint32_t zipCrc, struct stat64* st)
@@ -330,7 +289,7 @@
static NativeLibrariesIterator* create(ZipFileRO* zipFile, bool debuggable) {
void* cookie = nullptr;
// Do not specify a suffix to find both .so files and gdbserver.
- if (!zipFile->startIteration(&cookie, APK_LIB, nullptr /* suffix */)) {
+ if (!zipFile->startIteration(&cookie, APK_LIB.data(), nullptr /* suffix */)) {
return nullptr;
}
@@ -345,36 +304,11 @@
continue;
}
- // Make sure the filename is at least to the minimum library name size.
- const size_t fileNameLen = strlen(fileName);
- static const size_t minLength = APK_LIB_LEN + 2 + LIB_PREFIX_LEN + 1 + LIB_SUFFIX_LEN;
- if (fileNameLen < minLength) {
- continue;
+ const char* lastSlash = util::ValidLibraryPathLastSlash(fileName, false, mDebuggable);
+ if (lastSlash) {
+ mLastSlash = lastSlash;
+ break;
}
-
- const char* lastSlash = strrchr(fileName, '/');
- ALOG_ASSERT(lastSlash != nullptr, "last slash was null somehow for %s\n", fileName);
-
- // Skip directories.
- if (*(lastSlash + 1) == 0) {
- continue;
- }
-
- // Make sure the filename is safe.
- if (!isFilenameSafe(lastSlash + 1)) {
- continue;
- }
-
- if (!mDebuggable) {
- // Make sure the filename starts with lib and ends with ".so".
- if (strncmp(fileName + fileNameLen - LIB_SUFFIX_LEN, LIB_SUFFIX, LIB_SUFFIX_LEN)
- || strncmp(lastSlash, LIB_PREFIX, LIB_PREFIX_LEN)) {
- continue;
- }
- }
-
- mLastSlash = lastSlash;
- break;
}
return next;
@@ -543,7 +477,7 @@
}
const char* lastSlash = strrchr(fileName, '/');
const char* baseName = (lastSlash == nullptr) ? fileName : fileName + 1;
- if (isFilenameSafe(baseName)) {
+ if (util::isFilenameSafe(baseName)) {
zipFile->endIteration(cookie);
return BITCODE_PRESENT;
}
diff --git a/libs/androidfw/Android.bp b/libs/androidfw/Android.bp
index b1f327c..28bda72 100644
--- a/libs/androidfw/Android.bp
+++ b/libs/androidfw/Android.bp
@@ -55,6 +55,7 @@
host_supported: true,
srcs: [
"ApkAssets.cpp",
+ "ApkParsing.cpp",
"Asset.cpp",
"AssetDir.cpp",
"AssetManager.cpp",
@@ -160,6 +161,7 @@
// Actual tests.
"tests/ApkAssets_test.cpp",
+ "tests/ApkParsing_test.cpp",
"tests/AppAsLib_test.cpp",
"tests/Asset_test.cpp",
"tests/AssetManager2_test.cpp",
diff --git a/libs/androidfw/ApkParsing.cpp b/libs/androidfw/ApkParsing.cpp
new file mode 100644
index 0000000..cf4fbb9
--- /dev/null
+++ b/libs/androidfw/ApkParsing.cpp
@@ -0,0 +1,112 @@
+/*
+ * 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.
+ */
+
+#include "androidfw/ApkParsing.h"
+#include <algorithm>
+#include <array>
+#include <stdlib.h>
+#include <string_view>
+#include <sys/types.h>
+
+const std::string_view APK_LIB = "lib/";
+const size_t APK_LIB_LEN = APK_LIB.size();
+
+const std::string_view LIB_PREFIX = "/lib";
+const size_t LIB_PREFIX_LEN = LIB_PREFIX.size();
+
+const std::string_view LIB_SUFFIX = ".so";
+const size_t LIB_SUFFIX_LEN = LIB_SUFFIX.size();
+
+static const std::array<std::string_view, 2> abis = {"arm64-v8a", "x86_64"};
+
+namespace android::util {
+const char* ValidLibraryPathLastSlash(const char* fileName, bool suppress64Bit, bool debuggable) {
+ // Make sure the filename is at least to the minimum library name size.
+ const size_t fileNameLen = strlen(fileName);
+ static const size_t minLength = APK_LIB_LEN + 2 + LIB_PREFIX_LEN + 1 + LIB_SUFFIX_LEN;
+ if (fileNameLen < minLength) {
+ return nullptr;
+ }
+
+ const char* lastSlash = strrchr(fileName, '/');
+ if (!lastSlash) {
+ return nullptr;
+ }
+
+ // Skip directories.
+ if (*(lastSlash + 1) == 0) {
+ return nullptr;
+ }
+
+ // Make sure the filename is safe.
+ if (!isFilenameSafe(lastSlash + 1)) {
+ return nullptr;
+ }
+
+ // Make sure there aren't subdirectories
+ const char* abiOffset = fileName + APK_LIB_LEN;
+ const size_t abiSize = lastSlash - abiOffset;
+ if (memchr(abiOffset, '/', abiSize)) {
+ return nullptr;
+ }
+
+ if (!debuggable) {
+ // Make sure the filename starts with lib and ends with ".so".
+ if (strncmp(fileName + fileNameLen - LIB_SUFFIX_LEN, LIB_SUFFIX.data(), LIB_SUFFIX_LEN) != 0
+ || strncmp(lastSlash, LIB_PREFIX.data(), LIB_PREFIX_LEN) != 0) {
+ return nullptr;
+ }
+ }
+
+ // Don't include 64 bit versions if they are suppressed
+ if (suppress64Bit && std::find(abis.begin(), abis.end(), std::string_view(
+ fileName + APK_LIB_LEN, lastSlash - fileName - APK_LIB_LEN)) != abis.end()) {
+ return nullptr;
+ }
+
+ return lastSlash;
+}
+
+bool isFilenameSafe(const char* filename) {
+ off_t offset = 0;
+ for (;;) {
+ switch (*(filename + offset)) {
+ case 0:
+ // Null.
+ // If we've reached the end, all the other characters are good.
+ return true;
+
+ case 'A' ... 'Z':
+ case 'a' ... 'z':
+ case '0' ... '9':
+ case '+':
+ case ',':
+ case '-':
+ case '.':
+ case '/':
+ case '=':
+ case '_':
+ offset++;
+ break;
+
+ default:
+ // We found something that is not good.
+ return false;
+ }
+ }
+ // Should not reach here.
+}
+}
\ No newline at end of file
diff --git a/libs/androidfw/include/androidfw/ApkParsing.h b/libs/androidfw/include/androidfw/ApkParsing.h
new file mode 100644
index 0000000..194eaae
--- /dev/null
+++ b/libs/androidfw/include/androidfw/ApkParsing.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2023 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.
+ */
+#pragma once
+
+#include <string_view>
+#include <sys/types.h>
+
+extern const std::string_view APK_LIB;
+extern const size_t APK_LIB_LEN;
+
+namespace android::util {
+// Checks if filename is a valid library path and returns a pointer to the last slash in the path
+// if it is, nullptr otherwise
+const char* ValidLibraryPathLastSlash(const char* filename, bool suppress64Bit, bool debuggable);
+
+// Equivalent to android.os.FileUtils.isFilenameSafe
+bool isFilenameSafe(const char* filename);
+}
diff --git a/libs/androidfw/tests/ApkParsing_test.cpp b/libs/androidfw/tests/ApkParsing_test.cpp
new file mode 100644
index 0000000..4aab0a1
--- /dev/null
+++ b/libs/androidfw/tests/ApkParsing_test.cpp
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2023 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.
+ */
+
+#include "androidfw/ApkParsing.h"
+
+#include "android-base/test_utils.h"
+
+#include "TestHelpers.h"
+
+using ::testing::Eq;
+using ::testing::IsNull;
+using ::testing::NotNull;
+
+namespace android {
+TEST(ApkParsingTest, ValidArm64Path) {
+ const char* path = "lib/arm64-v8a/library.so";
+ auto lastSlash = util::ValidLibraryPathLastSlash(path, false, false);
+ ASSERT_THAT(lastSlash, NotNull());
+ ASSERT_THAT(lastSlash, Eq(path + 13));
+}
+
+TEST(ApkParsingTest, ValidArm64PathButSuppressed) {
+ const char* path = "lib/arm64-v8a/library.so";
+ auto lastSlash = util::ValidLibraryPathLastSlash(path, true, false);
+ ASSERT_THAT(lastSlash, IsNull());
+}
+
+TEST(ApkParsingTest, ValidArm32Path) {
+ const char* path = "lib/armeabi-v7a/library.so";
+ auto lastSlash = util::ValidLibraryPathLastSlash(path, false, false);
+ ASSERT_THAT(lastSlash, NotNull());
+ ASSERT_THAT(lastSlash, Eq(path + 15));
+}
+
+TEST(ApkParsingTest, InvalidMustStartWithLib) {
+ const char* path = "lib/arm64-v8a/random.so";
+ auto lastSlash = util::ValidLibraryPathLastSlash(path, false, false);
+ ASSERT_THAT(lastSlash, IsNull());
+}
+
+TEST(ApkParsingTest, InvalidMustEndInSo) {
+ const char* path = "lib/arm64-v8a/library.txt";
+ auto lastSlash = util::ValidLibraryPathLastSlash(path, false, false);
+ ASSERT_THAT(lastSlash, IsNull());
+}
+
+TEST(ApkParsingTest, InvalidCharacter) {
+ const char* path = "lib/arm64-v8a/lib#.so";
+ auto lastSlash = util::ValidLibraryPathLastSlash(path, false, false);
+ ASSERT_THAT(lastSlash, IsNull());
+}
+
+TEST(ApkParsingTest, InvalidSubdirectories) {
+ const char* path = "lib/arm64-v8a/anything/library.so";
+ auto lastSlash = util::ValidLibraryPathLastSlash(path, false, false);
+ ASSERT_THAT(lastSlash, IsNull());
+}
+}
\ No newline at end of file
diff --git a/tools/aapt2/dump/DumpManifest.cpp b/tools/aapt2/dump/DumpManifest.cpp
index d1957fb..c66f4e5 100644
--- a/tools/aapt2/dump/DumpManifest.cpp
+++ b/tools/aapt2/dump/DumpManifest.cpp
@@ -16,6 +16,8 @@
#include "DumpManifest.h"
+#include <androidfw/ApkParsing.h>
+
#include <algorithm>
#include <array>
#include <memory>
@@ -2729,19 +2731,25 @@
}));
supports_screen_ = screen ? screen : &default_screens;
- // Gather the supported architectures_ of the app
- std::set<std::string> architectures_from_apk;
+ bool has_renderscript_bitcode = false;
auto it = apk_->GetFileCollection()->Iterator();
while (it->HasNext()) {
- auto file_path = it->Next()->GetSource().path;
- if (file_path.starts_with("lib/")) {
- file_path = file_path.substr(4);
- size_t pos = file_path.find('/');
- if (pos != std::string::npos) {
- file_path = file_path.substr(0, pos);
- }
+ if (it->Next()->GetSource().path.ends_with(".bc")) {
+ has_renderscript_bitcode = true;
+ break;
+ }
+ }
- architectures_from_apk.insert(file_path);
+ // Gather the supported architectures_ of the app
+ std::set<std::string> architectures_from_apk;
+ it = apk_->GetFileCollection()->Iterator();
+ while (it->HasNext()) {
+ auto file_path = it->Next()->GetSource().path.c_str();
+
+ const char* last_slash =
+ android::util::ValidLibraryPathLastSlash(file_path, has_renderscript_bitcode, false);
+ if (last_slash) {
+ architectures_from_apk.insert(std::string(file_path + APK_LIB_LEN, last_slash));
}
}