Merge "servicemanager: restrict service name characters" into stage-aosp-master am: 8f4c72d3de am: e2ce3a1160

am: 4ac80bc0e9

Change-Id: I934053d546d0363b0da10379f0e2aef5d7dca2f4

2 files changed, 23 insertions, 2 deletions

diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index b88b67d138..6cfcf40970 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -63,6 +63,21 @@ Status ServiceManager::checkService(const std::string& name, sp<IBinder>* outBin
     return Status::ok();
 }
 
+bool isValidServiceName(const std::string& name) {
+    if (name.size() == 0) return false;
+    if (name.size() > 127) return false;
+
+    for (char c : name) {
+        if (c == '_' || c == '-' || c == '.') continue;
+        if (c >= 'a' && c <= 'z') continue;
+        if (c >= 'A' && c <= 'Z') continue;
+        if (c >= '0' && c <= '9') continue;
+        return false;
+    }
+
+    return true;
+}
+
 Status ServiceManager::addService(const std::string& name, const sp<IBinder>& binder, bool allowIsolated, int32_t dumpPriority) {
     auto ctx = mAccess->getCallingContext(name);
 
@@ -79,8 +94,8 @@ Status ServiceManager::addService(const std::string& name, const sp<IBinder>& bi
         return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT);
     }
 
-    // match legacy rules
-    if (name.size() == 0 || name.size() > 127) {
+    if (!isValidServiceName(name)) {
+        LOG(ERROR) << "Invalid service name: " << name;
         return Status::fromExceptionCode(Status::EX_ILLEGAL_ARGUMENT);
     }
 
diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index 812d5cacd5..25d2aa8bfa 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -82,6 +82,12 @@ TEST(AddService, TooLongNameDisallowed) {
         IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
 }
 
+TEST(AddService, WeirdCharactersDisallowed) {
+    auto sm = getPermissiveServiceManager();
+    EXPECT_FALSE(sm->addService("happy$foo$foo", getBinder(), false /*allowIsolated*/,
+        IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
+}
+
 TEST(AddService, AddNullServiceDisallowed) {
     auto sm = getPermissiveServiceManager();
     EXPECT_FALSE(sm->addService("foo", nullptr, false /*allowIsolated*/,