Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "StatsService allows uids to impersonate themselves"

Browse Source
This commit is contained in:
TreeHugger Robot
2018-12-19 21:20:57 +00:00
committed by Android (Google) Code Review
parent e8a25e3dbc d238657d69
commit a034692c81
3 changed files with 92 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

101
cmds/statsd/src/StatsService.cpp
View File

@@ -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
cmds/statsd/src/StatsService.h
View File

@@ -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
cmds/statsd/tests/StatsService_test.cpp
View File

@@ -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