Adding bufferSize Max to Guard Against Bad Inputs

Adding a guardrail to protect against config files that
specify unnecessarily large buffer sizes. Setting max buffer
size to 51200 bytes.

Bug: 240312562
Test: mma
Test: statsd_test64
Change-Id: Id1e6c4772f30df1072350be14f96e371ffbb497f

3 files changed, 41 insertions, 3 deletions

diff --git a/statsd/src/shell/ShellSubscriber.cpp b/statsd/src/shell/ShellSubscriber.cpp
index 35e64e47..eac65967 100644
--- a/statsd/src/shell/ShellSubscriber.cpp
+++ b/statsd/src/shell/ShellSubscriber.cpp
@@ -31,13 +31,13 @@ namespace statsd {
 
 const static int FIELD_ID_ATOM = 1;
 
-void ShellSubscriber::startNewSubscription(int in, int out, int timeoutSec) {
+bool ShellSubscriber::startNewSubscription(int in, int out, int timeoutSec) {
     int myToken = claimToken();
     VLOG("ShellSubscriber: new subscription %d has come in", myToken);
     mSubscriptionShouldEnd.notify_one();
 
     shared_ptr<SubscriptionInfo> mySubscriptionInfo = make_shared<SubscriptionInfo>(in, out);
-    if (!readConfig(mySubscriptionInfo)) return;
+    if (!readConfig(mySubscriptionInfo)) return false;
 
     {
         std::unique_lock<std::mutex> lock(mMutex);
@@ -50,6 +50,7 @@ void ShellSubscriber::startNewSubscription(int in, int out, int timeoutSec) {
         }
 
     }
+    return true;
 }
 
 void ShellSubscriber::spawnHelperThread(int myToken) {
@@ -87,6 +88,13 @@ bool ShellSubscriber::readConfig(shared_ptr<SubscriptionInfo> subscriptionInfo)
         return false;
     }
 
+    // Check bufferSize
+    if (bufferSize > (kMaxSizeKb * 1024)) {
+        ALOGE("Received config (%zu bytes) is larger than the max size (%zu bytes)", bufferSize,
+              (kMaxSizeKb * 1024));
+        return false;
+    }
+
     // Read the config.
     vector<uint8_t> buffer(bufferSize);
     if (!android::base::ReadFully(subscriptionInfo->mInputFd, buffer.data(), bufferSize)) {
diff --git a/statsd/src/shell/ShellSubscriber.h b/statsd/src/shell/ShellSubscriber.h
index 826ad31f..c6d11a3c 100644
--- a/statsd/src/shell/ShellSubscriber.h
+++ b/statsd/src/shell/ShellSubscriber.h
@@ -62,10 +62,14 @@ public:
     ShellSubscriber(sp<UidMap> uidMap, sp<StatsPullerManager> pullerMgr)
         : mUidMap(uidMap), mPullerMgr(pullerMgr){};
 
-    void startNewSubscription(int inFd, int outFd, int timeoutSec);
+    bool startNewSubscription(int inFd, int outFd, int timeoutSec);
 
     void onLogEvent(const LogEvent& event);
 
+    inline size_t getMaxSizeKb() const {
+        return kMaxSizeKb;
+    }
+
 private:
     struct PullInfo {
         PullInfo(const SimpleAtomMatcher& matcher, int64_t interval,
@@ -139,6 +143,9 @@ private:
     // when next to send a heartbeat.
     int64_t mLastWriteMs = 0;
     const int64_t kMsBetweenHeartbeats = 1000;
+
+    // Cap the buffer size of configs to guard against bad allocations
+    static constexpr size_t kMaxSizeKb = 50;
 };
 
 }  // namespace statsd
diff --git a/statsd/tests/shell/ShellSubscriber_test.cpp b/statsd/tests/shell/ShellSubscriber_test.cpp
index 37f5bf33..38b5fd15 100644
--- a/statsd/tests/shell/ShellSubscriber_test.cpp
+++ b/statsd/tests/shell/ShellSubscriber_test.cpp
@@ -142,6 +142,29 @@ TEST(ShellSubscriberTest, testPushedSubscription) {
     runShellTest(config, uidMap, pullerManager, pushedList, shellData);
 }
 
+TEST(ShellSubscriberTest, testMaxSizeGuard) {
+    sp<MockUidMap> uidMap = new NaggyMock<MockUidMap>();
+    sp<MockStatsPullerManager> pullerManager = new StrictMock<MockStatsPullerManager>();
+    sp<ShellSubscriber> shellClient = new ShellSubscriber(uidMap, pullerManager);
+
+    // set up 2 pipes for read/write config and data
+    int fds_config[2];
+    ASSERT_EQ(0, pipe(fds_config));
+
+    int fds_data[2];
+    ASSERT_EQ(0, pipe(fds_data));
+
+    // write invalid size of the config
+    size_t invalidBufferSize = (shellClient->getMaxSizeKb() * 1024) + 1;
+    write(fds_config[1], &invalidBufferSize, sizeof(invalidBufferSize));
+    close(fds_config[1]);
+    close(fds_data[0]);
+
+    EXPECT_FALSE(shellClient->startNewSubscription(fds_config[0], fds_data[1], /*timeoutSec=*/-1));
+    close(fds_config[0]);
+    close(fds_data[1]);
+}
+
 namespace {
 
 int kUid1 = 1000;