Have aapt2 check library names the same as package manager

Bug: 231297692
Test: Manual
Change-Id: I11a660969443aa90cf6b51a0947accca4231310f

6 files changed, 240 insertions, 82 deletions

diff --git a/core/jni/com_android_internal_content_NativeLibraryHelper.cpp b/core/jni/com_android_internal_content_NativeLibraryHelper.cpp
index 6762e45d0b79..2c819874973a 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 @@ enum install_status_t {
 
 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 @@ public:
     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 @@ public:
                 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 = 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;
+            const char* lastSlash = util::ValidLibraryPathLastSlash(fileName, false, mDebuggable);
+            if (lastSlash) {
+                mLastSlash = lastSlash;
+                break;
             }
-
-            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 @@ com_android_internal_content_NativeLibraryHelper_hasRenderscriptBitcode(JNIEnv *
         }
         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 b1f327c94f8e..28bda72bccdd 100644
--- a/libs/androidfw/Android.bp
+++ b/libs/androidfw/Android.bp
@@ -55,6 +55,7 @@ cc_library {
     host_supported: true,
     srcs: [
         "ApkAssets.cpp",
+        "ApkParsing.cpp",
         "Asset.cpp",
         "AssetDir.cpp",
         "AssetManager.cpp",
@@ -160,6 +161,7 @@ cc_test {
 
         // 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 000000000000..cf4fbb9b8462
--- /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 000000000000..194eaae8e12a
--- /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 000000000000..4aab0a146ecf
--- /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 d1957fb30cb9..c66f4e5b7c30 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 @@ bool ManifestExtractor::Extract(android::IDiagnostics* diag) {
       }));
   supports_screen_ = screen ? screen : &default_screens;
 
+  bool has_renderscript_bitcode = false;
+  auto it = apk_->GetFileCollection()->Iterator();
+  while (it->HasNext()) {
+    if (it->Next()->GetSource().path.ends_with(".bc")) {
+      has_renderscript_bitcode = true;
+      break;
+    }
+  }
+
   // Gather the supported architectures_ of the app
   std::set<std::string> architectures_from_apk;
-  auto it = apk_->GetFileCollection()->Iterator();
+  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);
-      }
+    auto file_path = it->Next()->GetSource().path.c_str();
 
-      architectures_from_apk.insert(file_path);
+    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));
     }
   }