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

Commit d238657d authored by Bookatz's avatar Bookatz
Browse files

StatsService allows uids to impersonate themselves

Previously, most StatsService calls only allow the caller to specify the
uid if it is Userdebug/EngBuild. This applied even if the caller was
just specifying its own UID; this cl allows such a case. It also allows
ROOT to impersonate SHELL.

Test: Manual testing on userdebug and userbuild devices.
Test: make statsd_test && adb sync data && adb shell data/nativetest64/statsd_test/statsd_test

Change-Id: I2685d13a8dc24d40b5dab8be9842f53be6888ba3
parent 67fb3ca9
Loading
Loading
Loading
Loading
+43 −58
Original line number Diff line number Diff line
@@ -466,23 +466,12 @@ status_t StatsService::cmd_trigger_broadcast(int out, Vector<String8>& args) {
        name.assign(args[1].c_str(), args[1].size());
        good = true;
    } else if (argCount == 3) {
        // If it's a userdebug or eng build, then the shell user can
        // impersonate other uids.
        if (mEngBuild) {
            const char* s = args[1].c_str();
            if (*s != '\0') {
                char* end = NULL;
                uid = strtol(s, &end, 0);
                if (*end == '\0') {
                    name.assign(args[2].c_str(), args[2].size());
                    good = true;
                }
            }
        } else {
            dprintf(out,
                    "The metrics can only be dumped for other UIDs on eng or userdebug "
                    "builds.\n");
        good = getUidFromArgs(args, 1, uid);
        if (!good) {
            dprintf(out, "Invalid UID. Note that the metrics can only be dumped for "
                         "other UIDs on eng or userdebug builds.\n");
        }
        name.assign(args[2].c_str(), args[2].size());
    }
    if (!good) {
        print_cmd_help(out);
@@ -518,23 +507,12 @@ status_t StatsService::cmd_config(int in, int out, int err, Vector<String8>& arg
                name.assign(args[2].c_str(), args[2].size());
                good = true;
            } else if (argCount == 4) {
                // If it's a userdebug or eng build, then the shell user can
                // impersonate other uids.
                if (mEngBuild) {
                    const char* s = args[2].c_str();
                    if (*s != '\0') {
                        char* end = NULL;
                        uid = strtol(s, &end, 0);
                        if (*end == '\0') {
                            name.assign(args[3].c_str(), args[3].size());
                            good = true;
                        }
                    }
                } else {
                    dprintf(err,
                            "The config can only be set for other UIDs on eng or userdebug "
                            "builds.\n");
                good = getUidFromArgs(args, 2, uid);
                if (!good) {
                    dprintf(err, "Invalid UID. Note that the config can only be set for "
                                 "other UIDs on eng or userdebug builds.\n");
                }
                name.assign(args[3].c_str(), args[3].size());
            } else if (argCount == 2 && args[1] == "remove") {
                good = true;
            }
@@ -612,23 +590,12 @@ status_t StatsService::cmd_dump_report(int out, const Vector<String8>& args) {
            name.assign(args[1].c_str(), args[1].size());
            good = true;
        } else if (argCount == 3) {
            // If it's a userdebug or eng build, then the shell user can
            // impersonate other uids.
            if (mEngBuild) {
                const char* s = args[1].c_str();
                if (*s != '\0') {
                    char* end = NULL;
                    uid = strtol(s, &end, 0);
                    if (*end == '\0') {
                        name.assign(args[2].c_str(), args[2].size());
                        good = true;
                    }
                }
            } else {
                dprintf(out,
                        "The metrics can only be dumped for other UIDs on eng or userdebug "
                        "builds.\n");
            good = getUidFromArgs(args, 1, uid);
            if (!good) {
                dprintf(out, "Invalid UID. Note that the metrics can only be dumped for "
                             "other UIDs on eng or userdebug builds.\n");
            }
            name.assign(args[2].c_str(), args[2].size());
        }
        if (good) {
            vector<uint8_t> data;
@@ -714,18 +681,14 @@ status_t StatsService::cmd_log_app_breadcrumb(int out, const Vector<String8>& ar
        state = atoi(args[2].c_str());
        good = true;
    } else if (argCount == 4) {
        uid = atoi(args[1].c_str());
        // If it's a userdebug or eng build, then the shell user can impersonate other uids.
        // Otherwise, the uid must match the actual caller's uid.
        if (mEngBuild || (uid >= 0 && (uid_t)uid == IPCThreadState::self()->getCallingUid())) {
            label = atoi(args[2].c_str());
            state = atoi(args[3].c_str());
            good = true;
        } else {
        good = getUidFromArgs(args, 1, uid);
        if (!good) {
            dprintf(out,
                    "Selecting a UID for writing AppBreadcrumb can only be done for other UIDs "
                    "on eng or userdebug builds.\n");
                    "Invalid UID. Note that selecting a UID for writing AppBreadcrumb can only be "
                    "done for other UIDs on eng or userdebug builds.\n");
        }
        label = atoi(args[2].c_str());
        state = atoi(args[3].c_str());
    }
    if (good) {
        dprintf(out, "Logging AppBreadcrumbReported(%d, %d, %d) to statslog.\n", uid, label, state);
@@ -792,6 +755,28 @@ status_t StatsService::cmd_print_logs(int out, const Vector<String8>& args) {
    }
}

bool StatsService::getUidFromArgs(const Vector<String8>& args, size_t uidArgIndex, int32_t& uid) {
    const char* s = args[uidArgIndex].c_str();
    if (*s == '\0') {
        return false;
    }
    char* endc = NULL;
    int64_t longUid = strtol(s, &endc, 0);
    if (*endc != '\0') {
        return false;
    }
    int32_t goodUid = static_cast<int32_t>(longUid);
    if (longUid < 0 || static_cast<uint64_t>(longUid) != static_cast<uid_t>(goodUid)) {
        return false;  // It was not of uid_t type.
    }
    uid = goodUid;

    int32_t callingUid = IPCThreadState::self()->getCallingUid();
    return mEngBuild // UserDebug/EngBuild are allowed to impersonate uids.
            || (callingUid == goodUid) // Anyone can 'impersonate' themselves.
            || (callingUid == AID_ROOT && goodUid == AID_SHELL); // ROOT can impersonate SHELL.
}

Status StatsService::informAllUidData(const vector<int32_t>& uid, const vector<int64_t>& version,
                                      const vector<String16>& version_string,
                                      const vector<String16>& app,
+10 −0
Original line number Diff line number Diff line
@@ -290,6 +290,15 @@ private:
     */
    status_t cmd_print_logs(int outFd, const Vector<String8>& args);

    /**
     * Writes the value of args[uidArgIndex] into uid.
     * Returns whether the uid is reasonable (type uid_t) and whether
     * 1. it is equal to the calling uid, or
     * 2. the device is mEngBuild, or
     * 3. the caller is AID_ROOT and the uid is AID_SHELL (i.e. ROOT can impersonate SHELL).
     */
    bool getUidFromArgs(const Vector<String8>& args, size_t uidArgIndex, int32_t& uid);

    /**
     * Adds a configuration after checking permissions and obtaining UID from binder call.
     */
@@ -340,6 +349,7 @@ private:
    FRIEND_TEST(StatsServiceTest, TestAddConfig_simple);
    FRIEND_TEST(StatsServiceTest, TestAddConfig_empty);
    FRIEND_TEST(StatsServiceTest, TestAddConfig_invalid);
    FRIEND_TEST(StatsServiceTest, TestGetUidFromArgs);
    FRIEND_TEST(PartialBucketE2eTest, TestCountMetricNoSplitOnNewApp);
    FRIEND_TEST(PartialBucketE2eTest, TestCountMetricSplitOnUpgrade);
    FRIEND_TEST(PartialBucketE2eTest, TestCountMetricSplitOnRemoval);
+39 −0
Original line number Diff line number Diff line
@@ -58,6 +58,45 @@ TEST(StatsServiceTest, TestAddConfig_invalid) {
            service.addConfigurationChecked(123, 12345, {serialized.begin(), serialized.end()}));
}

TEST(StatsServiceTest, TestGetUidFromArgs) {
    Vector<String8> args;
    args.push(String8("-1"));
    args.push(String8("0"));
    args.push(String8("1"));
    args.push(String8("9999999999999999999999999999999999"));
    args.push(String8("a1"));
    args.push(String8(""));

    int32_t uid;

    StatsService service(nullptr);
    service.mEngBuild = true;

    // "-1"
    EXPECT_FALSE(service.getUidFromArgs(args, 0, uid));

    // "0"
    EXPECT_TRUE(service.getUidFromArgs(args, 1, uid));
    EXPECT_EQ(0, uid);

    // "1"
    EXPECT_TRUE(service.getUidFromArgs(args, 2, uid));
    EXPECT_EQ(1, uid);

    // "999999999999999999"
    EXPECT_FALSE(service.getUidFromArgs(args, 3, uid));

    // "a1"
    EXPECT_FALSE(service.getUidFromArgs(args, 4, uid));

    // ""
    EXPECT_FALSE(service.getUidFromArgs(args, 5, uid));

    // For a non-userdebug, uid "1" cannot be impersonated.
    service.mEngBuild = false;
    EXPECT_FALSE(service.getUidFromArgs(args, 2, uid));
}

#else
GTEST_LOG_(INFO) << "This test does nothing.\n";
#endif