commit b1eee0beab855a35bba74b2b0f8c5152e5664069
author Muhammad Qureshi <muhammadq@google.com> Fri May 15 05:46:38 2020 -0700
committer Muhammad Qureshi <muhammadq@google.com> Fri May 15 19:13:56 2020 -0700
tree d8daff84b15e9c6d5044c64a8954a63610a41d5c
parent f681646d82f0824932cf9606456e20df4b0e8736

Don't create StateTrackers for whitelisted atoms

Whitelisted atoms can be logged from any app so integrity of StateTrackers
for those atoms cannot be guaranteed.

Fixes: 155522551
Test: statsd_test
Change-Id: Ifc64e9168ee5f1f8977b99534319bc65946630f0

cmds/statsd/src/metrics/MetricProducer.h

diff --git a/cmds/statsd/src/metrics/MetricProducer.h b/cmds/statsd/src/metrics/MetricProducer.h
index 91c98ea2..28563ad 100644
--- a/cmds/statsd/src/metrics/MetricProducer.h
+++ b/cmds/statsd/src/metrics/MetricProducer.h
@@ -442,7 +442,7 @@
     bool mIsActive;
 
     // The slice_by_state atom ids defined in statsd_config.
-    std::vector<int32_t> mSlicedStateAtoms;
+    const std::vector<int32_t> mSlicedStateAtoms;
 
     // Maps atom ids and state values to group_ids (<atom_id, <value, group_id>>).
     const std::unordered_map<int32_t, std::unordered_map<int, int64_t>> mStateGroupMap;

cmds/statsd/src/metrics/metrics_manager_util.cpp

diff --git a/cmds/statsd/src/metrics/metrics_manager_util.cpp b/cmds/statsd/src/metrics/metrics_manager_util.cpp
index 3ab44f4..210d382 100644
--- a/cmds/statsd/src/metrics/metrics_manager_util.cpp
+++ b/cmds/statsd/src/metrics/metrics_manager_util.cpp
@@ -791,10 +791,19 @@
         }
         noReportMetricIds.insert(no_report_metric);
     }
+
+    const set<int> whitelistedAtomIds(config.whitelisted_atom_ids().begin(),
+                                      config.whitelisted_atom_ids().end());
     for (const auto& it : allMetricProducers) {
         // Register metrics to StateTrackers
         for (int atomId : it->getSlicedStateAtoms()) {
-            StateManager::getInstance().registerListener(atomId, it);
+            // Register listener for non-whitelisted atoms only. Using whitelisted atom as a sliced
+            // state atom is not allowed.
+            if (whitelistedAtomIds.find(atomId) == whitelistedAtomIds.end()) {
+                StateManager::getInstance().registerListener(atomId, it);
+            } else {
+                return false;
+            }
         }
     }
     return true;

cmds/statsd/tests/MetricsManager_test.cpp

diff --git a/cmds/statsd/tests/MetricsManager_test.cpp b/cmds/statsd/tests/MetricsManager_test.cpp
index 15be0d2..ef8a34e 100644
--- a/cmds/statsd/tests/MetricsManager_test.cpp
+++ b/cmds/statsd/tests/MetricsManager_test.cpp
@@ -29,6 +29,7 @@
 #include "src/metrics/MetricProducer.h"
 #include "src/metrics/ValueMetricProducer.h"
 #include "src/metrics/metrics_manager_util.h"
+#include "src/state/StateManager.h"
 #include "statsd_test_util.h"
 
 using namespace testing;
@@ -615,6 +616,34 @@
     EXPECT_TRUE(metricsManager.checkLogCredentials(event));
 }
 
+TEST(MetricsManagerTest, TestWhitelistedAtomStateTracker) {
+    sp<UidMap> uidMap;
+    sp<StatsPullerManager> pullerManager = new StatsPullerManager();
+    sp<AlarmMonitor> anomalyAlarmMonitor;
+    sp<AlarmMonitor> periodicAlarmMonitor;
+
+    StatsdConfig config = buildGoodConfig();
+    config.add_allowed_log_source("AID_SYSTEM");
+    config.add_whitelisted_atom_ids(3);
+    config.add_whitelisted_atom_ids(4);
+
+    State state;
+    state.set_id(1);
+    state.set_atom_id(3);
+
+    *config.add_state() = state;
+
+    config.mutable_count_metric(0)->add_slice_by_state(state.id());
+
+    StateManager::getInstance().clear();
+
+    MetricsManager metricsManager(kConfigKey, config, timeBaseSec, timeBaseSec, uidMap,
+                                  pullerManager, anomalyAlarmMonitor, periodicAlarmMonitor);
+
+    EXPECT_EQ(0, StateManager::getInstance().getStateTrackersCount());
+    EXPECT_FALSE(metricsManager.isConfigValid());
+}
+
 }  // namespace statsd
 }  // namespace os
 }  // namespace android