Merge "aapt2: Limit length of package name and shared user id"

7 files changed, 176 insertions, 42 deletions

diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index f92c602fd761..b760958e0edc 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -1568,6 +1568,22 @@ class Linker {
     return true;
   }
 
+  ResourceEntry* ResolveTableEntry(LinkContext* context, ResourceTable* table,
+                                   Reference* reference) {
+    if (!reference || !reference->name) {
+      return nullptr;
+    }
+    auto name_ref = ResourceNameRef(reference->name.value());
+    if (name_ref.package.empty()) {
+      name_ref.package = context->GetCompilationPackage();
+    }
+    const auto search_result = table->FindResource(name_ref);
+    if (!search_result) {
+      return nullptr;
+    }
+    return search_result.value().entry;
+  }
+
   void AliasAdaptiveIcon(xml::XmlResource* manifest, ResourceTable* table) {
     const xml::Element* application = manifest->root->FindChild("", "application");
     if (!application) {
@@ -1582,22 +1598,13 @@ class Linker {
 
     // Find the icon resource defined within the application.
     const auto icon_reference = ValueCast<Reference>(icon->compiled_value.get());
-    if (!icon_reference || !icon_reference->name) {
-      return;
-    }
-
-    auto icon_name = ResourceNameRef(icon_reference->name.value());
-    if (icon_name.package.empty()) {
-      icon_name.package = context_->GetCompilationPackage();
-    }
-
-    const auto icon_entry_result = table->FindResource(icon_name);
-    if (!icon_entry_result) {
+    const auto icon_entry = ResolveTableEntry(context_, table, icon_reference);
+    if (!icon_entry) {
       return;
     }
 
     int icon_max_sdk = 0;
-    for (auto& config_value : icon_entry_result.value().entry->values) {
+    for (auto& config_value : icon_entry->values) {
       icon_max_sdk = (icon_max_sdk < config_value->config.sdkVersion)
           ? config_value->config.sdkVersion : icon_max_sdk;
     }
@@ -1608,22 +1615,13 @@ class Linker {
 
     // Find the roundIcon resource defined within the application.
     const auto round_icon_reference = ValueCast<Reference>(round_icon->compiled_value.get());
-    if (!round_icon_reference || !round_icon_reference->name) {
-      return;
-    }
-
-    auto round_icon_name = ResourceNameRef(round_icon_reference->name.value());
-    if (round_icon_name.package.empty()) {
-      round_icon_name.package = context_->GetCompilationPackage();
-    }
-
-    const auto round_icon_entry_result = table->FindResource(round_icon_name);
-    if (!round_icon_entry_result) {
+    const auto round_icon_entry = ResolveTableEntry(context_, table, round_icon_reference);
+    if (!round_icon_entry) {
       return;
     }
 
     int round_icon_max_sdk = 0;
-    for (auto& config_value : round_icon_entry_result.value().entry->values) {
+    for (auto& config_value : round_icon_entry->values) {
       round_icon_max_sdk = (round_icon_max_sdk < config_value->config.sdkVersion)
                      ? config_value->config.sdkVersion : round_icon_max_sdk;
     }
@@ -1634,7 +1632,7 @@ class Linker {
     }
 
     // Add an equivalent v26 entry to the roundIcon for each v26 variant of the regular icon.
-    for (auto& config_value : icon_entry_result.value().entry->values) {
+    for (auto& config_value : icon_entry->values) {
       if (config_value->config.sdkVersion < SDK_O) {
         continue;
       }
@@ -1645,12 +1643,62 @@ class Linker {
                                                      << "\" for round icon compatibility");
 
       auto value = icon_reference->Clone(&table->string_pool);
-      auto round_config_value = round_icon_entry_result.value().entry->FindOrCreateValue(
-          config_value->config, config_value->product);
+      auto round_config_value =
+          round_icon_entry->FindOrCreateValue(config_value->config, config_value->product);
       round_config_value->value.reset(value);
     }
   }
 
+  bool VerifySharedUserId(xml::XmlResource* manifest, ResourceTable* table) {
+    const xml::Element* manifest_el = xml::FindRootElement(manifest->root.get());
+    if (manifest_el == nullptr) {
+      return true;
+    }
+    if (!manifest_el->namespace_uri.empty() || manifest_el->name != "manifest") {
+      return true;
+    }
+    const xml::Attribute* attr = manifest_el->FindAttribute(xml::kSchemaAndroid, "sharedUserId");
+    if (!attr) {
+      return true;
+    }
+    const auto validate = [&](const std::string& shared_user_id) -> bool {
+      if (util::IsAndroidSharedUserId(context_->GetCompilationPackage(), shared_user_id)) {
+        return true;
+      }
+      DiagMessage error_msg(manifest_el->line_number);
+      error_msg << "attribute 'sharedUserId' in <manifest> tag is not a valid shared user id: '"
+                << shared_user_id << "'";
+      if (options_.manifest_fixer_options.warn_validation) {
+        // Treat the error only as a warning.
+        context_->GetDiagnostics()->Warn(error_msg);
+        return true;
+      }
+      context_->GetDiagnostics()->Error(error_msg);
+      return false;
+    };
+    // If attr->compiled_value is not null, check if it is a ref
+    if (attr->compiled_value) {
+      const auto ref = ValueCast<Reference>(attr->compiled_value.get());
+      if (ref == nullptr) {
+        return true;
+      }
+      const auto shared_user_id_entry = ResolveTableEntry(context_, table, ref);
+      if (!shared_user_id_entry) {
+        return true;
+      }
+      for (const auto& value : shared_user_id_entry->values) {
+        const auto str_value = ValueCast<String>(value->value.get());
+        if (str_value != nullptr && !validate(*str_value->value)) {
+          return false;
+        }
+      }
+      return true;
+    }
+
+    // Fallback to checking the raw value
+    return validate(attr->value);
+  }
+
   // Writes the AndroidManifest, ResourceTable, and all XML files referenced by the ResourceTable
   // to the IArchiveWriter.
   bool WriteApk(IArchiveWriter* writer, proguard::KeepSet* keep_set, xml::XmlResource* manifest,
@@ -1672,6 +1720,11 @@ class Linker {
     // See (b/34829129)
     AliasAdaptiveIcon(manifest, table);
 
+    // Verify the shared user id here to handle the case of reference value.
+    if (!VerifySharedUserId(manifest, table)) {
+      return false;
+    }
+
     ResourceFileFlattenerOptions file_flattener_options;
     file_flattener_options.keep_raw_values = keep_raw_values;
     file_flattener_options.do_not_compress_anything = options_.do_not_compress_anything;
diff --git a/tools/aapt2/link/ManifestFixer.cpp b/tools/aapt2/link/ManifestFixer.cpp
index fd3a4c035076..c03661ca2366 100644
--- a/tools/aapt2/link/ManifestFixer.cpp
+++ b/tools/aapt2/link/ManifestFixer.cpp
@@ -163,7 +163,8 @@ static bool AutoGenerateIsFeatureSplit(xml::Element* el, SourcePathDiagnostics*
   return true;
 }
 
-static bool VerifyManifest(xml::Element* el, SourcePathDiagnostics* diag) {
+static bool VerifyManifest(xml::Element* el, xml::XmlActionExecutorPolicy policy,
+                           SourcePathDiagnostics* diag) {
   xml::Attribute* attr = el->FindAttribute({}, "package");
   if (!attr) {
     diag->Error(DiagMessage(el->line_number)
@@ -174,10 +175,16 @@ static bool VerifyManifest(xml::Element* el, SourcePathDiagnostics* diag) {
                 << "attribute 'package' in <manifest> tag must not be a reference");
     return false;
   } else if (!util::IsAndroidPackageName(attr->value)) {
-    diag->Error(DiagMessage(el->line_number)
-                << "attribute 'package' in <manifest> tag is not a valid Android package name: '"
-                << attr->value << "'");
-    return false;
+    DiagMessage error_msg(el->line_number);
+    error_msg << "attribute 'package' in <manifest> tag is not a valid Android package name: '"
+              << attr->value << "'";
+    if (policy == xml::XmlActionExecutorPolicy::kAllowListWarning) {
+      // Treat the error only as a warning.
+      diag->Warn(error_msg);
+    } else {
+      diag->Error(error_msg);
+      return false;
+    }
   }
 
   attr = el->FindAttribute({}, "split");
diff --git a/tools/aapt2/util/Util.cpp b/tools/aapt2/util/Util.cpp
index ef33c3463a81..28330db966af 100644
--- a/tools/aapt2/util/Util.cpp
+++ b/tools/aapt2/util/Util.cpp
@@ -38,6 +38,11 @@ using ::android::StringPiece16;
 namespace aapt {
 namespace util {
 
+// Package name and shared user id would be used as a part of the file name.
+// Limits size to 223 and reserves 32 for the OS.
+// See frameworks/base/core/java/android/content/pm/parsing/ParsingPackageUtils.java
+constexpr static const size_t kMaxPackageNameSize = 223;
+
 static std::vector<std::string> SplitAndTransform(
     const StringPiece& str, char sep, const std::function<char(char)>& f) {
   std::vector<std::string> parts;
@@ -169,9 +174,21 @@ static int IsAndroidNameImpl(const StringPiece& str) {
 }
 
 bool IsAndroidPackageName(const StringPiece& str) {
+  if (str.size() > kMaxPackageNameSize) {
+    return false;
+  }
   return IsAndroidNameImpl(str) > 1 || str == "android";
 }
 
+bool IsAndroidSharedUserId(const android::StringPiece& package_name,
+                           const android::StringPiece& shared_user_id) {
+  if (shared_user_id.size() > kMaxPackageNameSize) {
+    return false;
+  }
+  return shared_user_id.empty() || IsAndroidNameImpl(shared_user_id) > 1 ||
+         package_name == "android";
+}
+
 bool IsAndroidSplitName(const StringPiece& str) {
   return IsAndroidNameImpl(str) > 0;
 }
diff --git a/tools/aapt2/util/Util.h b/tools/aapt2/util/Util.h
index a956957eace8..c77aca31a810 100644
--- a/tools/aapt2/util/Util.h
+++ b/tools/aapt2/util/Util.h
@@ -81,6 +81,7 @@ bool IsJavaPackageName(const android::StringPiece& str);
 // - First character of each component (separated by '.') must be an ASCII letter.
 // - Subsequent characters of a component can be ASCII alphanumeric or an underscore.
 // - Package must contain at least two components, unless it is 'android'.
+// - The maximum package name length is 223.
 bool IsAndroidPackageName(const android::StringPiece& str);
 
 // Tests that the string is a valid Android split name.
@@ -88,6 +89,15 @@ bool IsAndroidPackageName(const android::StringPiece& str);
 // - Subsequent characters of a component can be ASCII alphanumeric or an underscore.
 bool IsAndroidSplitName(const android::StringPiece& str);
 
+// Tests that the string is a valid Android shared user id.
+// - First character of each component (separated by '.') must be an ASCII letter.
+// - Subsequent characters of a component can be ASCII alphanumeric or an underscore.
+// - Must contain at least two components, unless package name is 'android'.
+// - The maximum shared user id length is 223.
+// - Treat empty string as valid, it's the case of no shared user id.
+bool IsAndroidSharedUserId(const android::StringPiece& package_name,
+                           const android::StringPiece& shared_user_id);
+
 // Converts the class name to a fully qualified class name from the given
 // `package`. Ex:
 //
diff --git a/tools/aapt2/util/Util_test.cpp b/tools/aapt2/util/Util_test.cpp
index d4e3bec24bd1..4ebcb115306f 100644
--- a/tools/aapt2/util/Util_test.cpp
+++ b/tools/aapt2/util/Util_test.cpp
@@ -27,6 +27,17 @@ using ::testing::SizeIs;
 
 namespace aapt {
 
+// Test that a max package name size 223 is valid.
+static const std::string kMaxPackageName =
+    "com.foo.nameRw8ajIGbYmqPuO0K7TYJFsI2pjlDAS0pYOYQlJvtQux"
+    "SoBKV1hMyNh4XfmcMj8OgPHfFaTXeKEHFMdGQHpw9Dz9Uqr8h1krgJLRv2aXyPCsGdVwBJzfZ4COVRiX3sc9O"
+    "CUrTTvZe6wXlgKb5Qz5qdkTBZ5euzGeoyZwestDTBIgT5exAl5efnznwzceS7VsIntgY10UUQvaoTsLBO6l";
+// Test that a long package name size 224 is invalid.
+static const std::string kLongPackageName =
+    "com.foo.nameRw8ajIGbYmqPuO0K7TYJFsI2pjlDAS0pYOYQlJvtQu"
+    "xSoBKV1hMyNh4XfmcMj8OgPHfFaTXeKEHFMdGQHpw9Dz9Uqr8h1krgJLRv2aXyPCsGdVwBJzfZ4COVRiX3sc9O"
+    "CUrTTvZe6wXlgKb5Qz5qdkTBZ5euzGeoyZwestDTBIgT5exAl5efnznwzceS7VsIntgY10UUQvaoTsLBO6le";
+
 TEST(UtilTest, TrimOnlyWhitespace) {
   const StringPiece trimmed = util::TrimWhitespace("\n        ");
   EXPECT_TRUE(trimmed.empty());
@@ -108,6 +119,7 @@ TEST(UtilTest, IsAndroidPackageName) {
   EXPECT_TRUE(util::IsAndroidPackageName("com.foo.test_thing"));
   EXPECT_TRUE(util::IsAndroidPackageName("com.foo.testing_thing_"));
   EXPECT_TRUE(util::IsAndroidPackageName("com.foo.test_99_"));
+  EXPECT_TRUE(util::IsAndroidPackageName(kMaxPackageName));
 
   EXPECT_FALSE(util::IsAndroidPackageName("android._test"));
   EXPECT_FALSE(util::IsAndroidPackageName("com"));
@@ -116,6 +128,27 @@ TEST(UtilTest, IsAndroidPackageName) {
   EXPECT_FALSE(util::IsAndroidPackageName(".android"));
   EXPECT_FALSE(util::IsAndroidPackageName(".."));
   EXPECT_FALSE(util::IsAndroidPackageName("cøm.foo"));
+  EXPECT_FALSE(util::IsAndroidPackageName(kLongPackageName));
+}
+
+TEST(UtilTest, IsAndroidSharedUserId) {
+  EXPECT_TRUE(util::IsAndroidSharedUserId("android", "foo"));
+  EXPECT_TRUE(util::IsAndroidSharedUserId("com.foo", "android.test"));
+  EXPECT_TRUE(util::IsAndroidSharedUserId("com.foo", "com.foo"));
+  EXPECT_TRUE(util::IsAndroidSharedUserId("com.foo", "com.foo.test_thing"));
+  EXPECT_TRUE(util::IsAndroidSharedUserId("com.foo", "com.foo.testing_thing_"));
+  EXPECT_TRUE(util::IsAndroidSharedUserId("com.foo", "com.foo.test_99_"));
+  EXPECT_TRUE(util::IsAndroidSharedUserId("com.foo", ""));
+  EXPECT_TRUE(util::IsAndroidSharedUserId("com.foo", kMaxPackageName));
+
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", "android._test"));
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", "com"));
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", "_android"));
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", "android."));
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", ".android"));
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", ".."));
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", "cøm.foo"));
+  EXPECT_FALSE(util::IsAndroidSharedUserId("com.foo", kLongPackageName));
 }
 
 TEST(UtilTest, FullyQualifiedClassName) {
diff --git a/tools/aapt2/xml/XmlActionExecutor.cpp b/tools/aapt2/xml/XmlActionExecutor.cpp
index fab17c949dd8..ea42d26358a8 100644
--- a/tools/aapt2/xml/XmlActionExecutor.cpp
+++ b/tools/aapt2/xml/XmlActionExecutor.cpp
@@ -21,23 +21,34 @@ using ::android::StringPiece;
 namespace aapt {
 namespace xml {
 
-static bool wrapper_one(XmlNodeAction::ActionFunc& f, Element* el, SourcePathDiagnostics*) {
+static bool wrapper_one(const XmlNodeAction::ActionFunc& f, Element* el,
+                        const XmlActionExecutorPolicy& policy, SourcePathDiagnostics*) {
   return f(el);
 }
 
-static bool wrapper_two(XmlNodeAction::ActionFuncWithDiag& f, Element* el,
-                        SourcePathDiagnostics* diag) {
+static bool wrapper_two(const XmlNodeAction::ActionFuncWithDiag& f, Element* el,
+                        const XmlActionExecutorPolicy& policy, SourcePathDiagnostics* diag) {
   return f(el, diag);
 }
 
+static bool wrapper_three(const XmlNodeAction::ActionFuncWithPolicyAndDiag& f, Element* el,
+                          const XmlActionExecutorPolicy& policy, SourcePathDiagnostics* diag) {
+  return f(el, policy, diag);
+}
+
 void XmlNodeAction::Action(XmlNodeAction::ActionFunc f) {
-  actions_.emplace_back(std::bind(
-      wrapper_one, std::move(f), std::placeholders::_1, std::placeholders::_2));
+  actions_.emplace_back(std::bind(wrapper_one, std::move(f), std::placeholders::_1,
+                                  std::placeholders::_2, std::placeholders::_3));
 }
 
 void XmlNodeAction::Action(XmlNodeAction::ActionFuncWithDiag f) {
-  actions_.emplace_back(std::bind(
-      wrapper_two, std::move(f), std::placeholders::_1, std::placeholders::_2));
+  actions_.emplace_back(std::bind(wrapper_two, std::move(f), std::placeholders::_1,
+                                  std::placeholders::_2, std::placeholders::_3));
+}
+
+void XmlNodeAction::Action(XmlNodeAction::ActionFuncWithPolicyAndDiag f) {
+  actions_.emplace_back(std::bind(wrapper_three, std::move(f), std::placeholders::_1,
+                                  std::placeholders::_2, std::placeholders::_3));
 }
 
 static void PrintElementToDiagMessage(const Element* el, DiagMessage* msg) {
@@ -51,8 +62,8 @@ static void PrintElementToDiagMessage(const Element* el, DiagMessage* msg) {
 bool XmlNodeAction::Execute(XmlActionExecutorPolicy policy, std::vector<StringPiece>* bread_crumb,
                             SourcePathDiagnostics* diag, Element* el) const {
   bool error = false;
-  for (const ActionFuncWithDiag& action : actions_) {
-    error |= !action(el, diag);
+  for (const ActionFuncWithPolicyAndDiag& action : actions_) {
+    error |= !action(el, policy, diag);
   }
 
   for (Element* child_el : el->GetChildElements()) {
diff --git a/tools/aapt2/xml/XmlActionExecutor.h b/tools/aapt2/xml/XmlActionExecutor.h
index a0ad1dadeddf..78c43345deb7 100644
--- a/tools/aapt2/xml/XmlActionExecutor.h
+++ b/tools/aapt2/xml/XmlActionExecutor.h
@@ -49,6 +49,8 @@ enum class XmlActionExecutorPolicy {
 // holds XmlNodeActions for child XML nodes.
 class XmlNodeAction {
  public:
+  using ActionFuncWithPolicyAndDiag =
+      std::function<bool(Element*, XmlActionExecutorPolicy, SourcePathDiagnostics*)>;
   using ActionFuncWithDiag = std::function<bool(Element*, SourcePathDiagnostics*)>;
   using ActionFunc = std::function<bool(Element*)>;
 
@@ -61,6 +63,7 @@ class XmlNodeAction {
   // Add an action to be performed at this XmlNodeAction.
   void Action(ActionFunc f);
   void Action(ActionFuncWithDiag);
+  void Action(ActionFuncWithPolicyAndDiag);
 
  private:
   friend class XmlActionExecutor;
@@ -69,7 +72,7 @@ class XmlNodeAction {
                SourcePathDiagnostics* diag, Element* el) const;
 
   std::map<std::string, XmlNodeAction> map_;
-  std::vector<ActionFuncWithDiag> actions_;
+  std::vector<ActionFuncWithPolicyAndDiag> actions_;
 };
 
 // Allows the definition of actions to execute at specific XML elements defined by their hierarchy.