Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 905e2e85 authored by Steven Moreland's avatar Steven Moreland
Browse files

servicemanager: restrict service name characters 

This is in order to, in the future, potentially have special policy
like:
vendor$vendor_service u:object_r:...:s0
universal$hal_foo_service u:object_r:...:s0

Currently, there are no special characters that could be used for this
purpose.

How safe is this? In all context files in our internal tree, only the
following characters are used in service names:
-._12aAbBcCdDeEfFgGhHiIjkKlmMnNoOpPqQrsStTuUvwWxyz

'.' appears everywhere to mean '\.'.

Test: servicemanager_test
Bug: 136027762
Change-Id: Idd6698a68bbbe825dd5a25d92baf81fae473ea2a
parent 94f6b215

cmds/servicemanager/ServiceManager.cpp

+17 −2
Original line number Diff line number Diff line
@@ -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);

    }

cmds/servicemanager/test_sm.cpp

+6 −0
Original line number Diff line number Diff line
@@ -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*/,